Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

mirrorring relative symlinks with ../.. in them which are wrong... #1270

Closed
petersilva opened this issue Oct 22, 2024 · 5 comments · Fixed by #1272
Closed

mirrorring relative symlinks with ../.. in them which are wrong... #1270

petersilva opened this issue Oct 22, 2024 · 5 comments · Fixed by #1272
Assignees
@petersilva
Labels
Blocker bad enough that no new release should happen bug Something isn't working HPC related to hich performance computing mirroing use case

Comments

@petersilva
Copy link
Contributor

so.. when mirroring with libsr3shim:

shim posting:

realpathFilter on
realpathPost on
realpathAdjust -1

subscriber:

baseDir /source
directory /dest

now there are paths which are /home/user/excellent (where excellent is a link into /source/excellent) so the
path of operations would be:

cd /home/user/excellent
mv a b
mkdir oneDirDeep
mkdir oneDirDeep/twoDirDeep
echo "hoho" >>oneDirDeep/twoDirDeep/2dd_file1
ln -s 2dd_file1 oneDirDeep/twoDirDeep/link_to_2dd_file1
mv  oneDirDeep/twoDirDeep/link_to_2dd_file1  oneDirDeep/twoDirDeep/renamed_link_to_2dd_file1

you end up with links with (wrong) extra ../../ in them.
also rename... maybe same issue maybe different.
@petersilva petersilva self-assigned this Oct 22, 2024
@petersilva petersilva added bug Something isn't working Blocker bad enough that no new release should happen labels Oct 22, 2024
@petersilva
Copy link
Contributor Author

user found this...

petersilva added a commit that referenced this issue Oct 22, 2024
@petersilva
symlinks are always relative to location of file.
924ad2f 
working on #1270.

The relative path conversion is actually different for rename vs. link.
the rename needs to take into account where in the tree it is,
but the link is always from the destination directory (where the link
is.)  So the processing of 'link' and 'rename' is actually different.

I need more testing, I think link is correct now.
@petersilva
Copy link
Contributor Author

OK, what I think is going on:

  • when you are processing file renames, removes, and symlinks. the cwd is the directory. So the rename's relative path needs to be adjusted to make it relative to directory ...
  • I applied that relative path adjustment logic to all fileOps ('hlink', 'link', 'rename') but... 'link' is a symbolic link. the argument supplied is just written into the link, so no adjustment is needed.
  • so 'hlink' (hard links) and 'rename' do need adjustment, but symbolic links do not.

@petersilva
Copy link
Contributor Author

I added some test cases to sr_insects to cover this case, and they are fixed by the proposed patch.

@petersilva
Copy link
Contributor Author

petersilva commented Oct 22, 2024 •
edited
Loading

Question... would it not be simpler to just chdir into the directory, and do relative ops from there...
then no adjustment would be needed. hmm.

I look at the logic in do_download... and looks like it is doing a chdir... before the fileOp... so I don't understand
why the relative path adjustment is needed at all. but if I remove if for renames, my test cases break.

@petersilva petersilva mentioned this issue Oct 22, 2024
Merged
petersilva added a commit that referenced this issue Oct 22, 2024
@petersilva
working on #1270 and sarrac #169 together.
d4ae32a 
It looks like sarrac #169 was being mostly hidden/corrected by
inexplicable logic here. Once #169 is corrected, the strange
adjustments to the relative path can be removed.
@petersilva
Copy link
Contributor Author

After the problem with sarrac was corrected, now tests pass when all weird processing of relative rename and link passes all tests.

@petersilva petersilva mentioned this issue Oct 25, 2024
Merged
@petersilva petersilva added the HPC related to hich performance computing mirroing use case label Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@petersilva petersilva

Labels
Blocker bad enough that no new release should happen bug Something isn't working HPC related to hich performance computing mirroing use case
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Issue1270 - fixing relative symlinks with (inappropriate) ../../ in them... MetPX/sarracenia
1 participant
@petersilva