Fileinstall: Permit directories containing broken symlinks#2948
Fileinstall: Permit directories containing broken symlinks#2948wxtim merged 1 commit intometomi:masterfrom
Conversation
wxtim
force-pushed
the
fix.install_broken_symlinks_in_directories
branch
from
be40402 to
554a546
Compare
wxtim
force-pushed
the
fix.install_broken_symlinks_in_directories
branch
from
a4984c3 to
e0ae6c3
Compare
| ``~/.ssh/config`` to specify the user ID for logging into ``HOST`` | ||
| if required. | ||
|
|
||
| .. note:: |
There was a problem hiding this comment.
IMO this could probably use a larger re-write, but simply getting this bit of info into the docs is a start.
There was a problem hiding this comment.
There was no prior unit testing at all for this module.
| run_pass "${TEST_KEY_BASE}-git" rose app-run --config=../config | ||
| file_grep_fail "${TEST_KEY_BASE}-git.err" \ | ||
| "No such file" \ | ||
| "${TEST_KEY_BASE}-git.err" |
There was a problem hiding this comment.
Ideally there should be an SVN test here too.
wxtim
force-pushed
the
fix.install_broken_symlinks_in_directories
branch
from
8a6fc88 to
b13feb6
Compare
| if os.path.islink(source) and not os.path.exists(source): | ||
| # If the source is a broken link within a directory | ||
| # install without failing (unlike a single file) | ||
| checksum = os.path.realpath(source) | ||
| mode = os.lstat(source).st_mode | ||
| else: | ||
| checksum = checksum_func(source, name) | ||
| mode = os.stat(os.path.realpath(source)).st_mode |
There was a problem hiding this comment.
When installing directories using Git or Rsync broken symlinks will be copied.
This change targets the get_checksum function in general, not the git / rsync methods specificity.
Are we sure this method is only called for these methods, or should we pass the method name through to this function to ensure this logic is only activated when intended?
Also, this impacts Git, does it also impact SVN?
There was a problem hiding this comment.
Are we sure this method is only called for these methods, or should we pass the method name through to this function to ensure this logic is only activated when intended?
I think that it makes sense to be consistent. The only other place this logic is used is Rose arch. I think it makes sense there too.
Also, this impacts Git, does it also impact SVN?
I hope so - I would have tested it (and still can if you insist) but setting up the test would have taken extra time.
@dpmatthews - what do you think?
There was a problem hiding this comment.
It should (and does) apply to all cases including SVN (I've tested this)
There was a problem hiding this comment.
@oliver-sanders please resolve if you're happy...
Add some documentation a little more unit test coverage of some of this Update rsync_remote_check.py Response to review
wxtim
force-pushed
the
fix.install_broken_symlinks_in_directories
branch
from
2acf76f to
e2e9a69
Compare
4 participants
When complete will close #2946 .
After discussion with @dpmatthews we think that broken links inside directories being installed should not cause a failure.