Skip to content

Prevent circular symlinks from overwriting atmosphere files - #992

Closed
trhille wants to merge 3 commits into
MPAS-Dev:mainfrom
trhille:landice/fix_circular_symlinks
Closed

trhille wants to merge 3 commits into
MPAS-Dev:mainfrom
trhille:landice/fix_circular_symlinks

Conversation

@trhille

@trhille trhille commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

The previous logic allowed symlinks to overwrite original files when processing was run multiple times. This new logic should prevent that.

Checklist

  • Document (in a comment titled Testing in this PR) any testing that was used to verify the changes

The previous logic allowed symlinks to overwrite original files
when processing was run multiple times. This new logic should
prevent that.
@trhille trhille added this to the ISMIP7 milestone Sep 21, 2026
trhille and others added 2 commits September 24, 2026 13:01
The previous fix (dc3decf) added os.path.islink(src) to skip symlinks
on the source side, but did not prevent the case where the destination
directory itself resolves to the source directory (e.g., when OCX_main
is a symlink to OCX, or when OCX_main/atmosphere is a symlink to
OCX/atmosphere).

This change adds two realpath-based guards:
1. Directory-level: Skip the entire mirror operation if the destination
   directory resolves to the source directory.
2. File-level: Skip individual files if the destination file would
   resolve to the source file.

Both guards use os.path.realpath() which fully resolves symlink chains,
so the original atmosphere files can never be overwritten by the
mirroring logic, regardless of directory aliasing.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@trhille
trhille force-pushed the landice/fix_circular_symlinks branch from 6c76828 to a2b2965 Compare September 24, 2026 19:29
@trhille

trhille commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator Author

Closing this in favor of #997, which contains these changes in the refactored code.

@trhille trhille closed this Sep 25, 2026
@trhille trhille mentioned this pull request Sep 25, 2026
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant