`rimport` destroys the inputdata link when the path given goes through a symlink
- Dominant language
- Python
- Stars
- 0
- Forks
- 2
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 2
Description
(Identified and written by Claude; I haven't yet verified.)
## Summary
If you name a file through a symlink that points into the inputdata tree, `rimport`
stages the file correctly but then replaces the original with a symlink **pointing at
itself**. The published file becomes unreadable (`ELOOP`), and `rimport` exits 1.
The staged copy is fine, so no data is lost — but the path everyone reads from is broken
until someone repairs it by hand.
## Impact
- The inputdata path is left as a self-referential symlink: `cat` reports
`Too many levels of symbolic links`, and `Path.exists()` returns `False`.
- Anything resolving that path — a CESM run, a download check, a later `rimport` — sees a
broken file.
- It is silent about the real cause. The run reports `Error relinking during rimport`,
which reads like a transient failure rather than a corrupted link.
Severity is bounded by two things: the staged copy under the staging root is intact and
correct, and the failure is detected (exit 1) rather than passing as success. It is a
recoverable break, not data loss.
## Reproduction
```bash
W=$(mktemp -d)
mkdir -p "$W/inputdata/lnd" "$W/staging"
echo data > "$W/inputdata/lnd/a.nc"
ln -s "$W/inputdata" "$W/plink" # a symlink pointing at the inputdata root
RIMPORT_STAGING="$W/staging" RIMPORT_SKIP_USER_CHECK=1 \
rimport -inputdata "$W/inputdata" "$W/plink/lnd/a.nc"
```
Observed:
```
'/plink/lnd/a.nc':
[rimport] staged /plink/lnd/a.nc -> /staging/lnd/a.nc
Deleted original file: /plink/lnd/a.nc
Created symbolic link: /plink/lnd/a.nc -> /staging/../plink/lnd/a.nc
rimport: error processing /plink/lnd/a.nc: Error relinking during rimport
EXIT=1
```
`/staging/../plink/lnd/a.nc` normalizes to `/plink/lnd/a.nc`, which is the symlink
that was just created. Afterwards:
```
$ ls -l "$W/inputdata/lnd/a.nc"
... a.nc -> /staging/../plink/lnd/a.nc
$ cat "$W/inputdata/lnd/a.nc"
cat: ...: Too many levels of symbolic links
$ cat "$W/staging/lnd/a.nc"
data # the staged copy is correct
```
Expected: either the same result as naming the file directly (staged, and the original
replaced with a working symlink into staging), or a clean pre-flight rejection. Not a
half-completed publish that breaks the path.
## Root cause
Two places compute "where does this file live relative to the inputdata root", and they
disagree when the path reaches the tree through a symlink.
| Site | Computation | Result for the repro |
|------|-------------|----------------------|
| `rimport:526` (`stage_data`) | `src.resolve().relative_to(inputdata_root.resolve())` — **resolved** | `lnd/a.nc` → stages to `/staging/lnd/a.nc` ✅ |
| `relink.py:244` (`replace_one_file_with_symlink`) | `os.path.relpath(file_path, inputdata_root)` — **lexical** | `../plink/lnd/a.nc` → links to `/staging/../plink/lnd/a.nc` ❌ |
`os.path.relpath` does no filesystem lookup, so it cannot see that `plink` and `inputdata`
are the same directory. It escapes the root with `..`, and joining that onto the staging
root walks straight back out to the original path.
`relink`'s own existence check does not catch it: at that moment `link_target` still
resolves to the original file, which does exist, so it proceeds to delete the original and
link over it. `check_relink` (`rimport:400-402`) notices afterwards and raises — after the
damage is done.
## Suggested fix
Make the two agree. The narrower change is in `relink.py:244`, which also covers `relink`
used standalone:
```python
relative_path = os.path.relpath(
os.path.realpath(file_path), os.path.realpath(inputdata_root)
)
```
This was tried against the reproduction above: the run then exits 0 and produces
`a.nc -> /staging/lnd/a.nc`, which reads correctly. The existing suite (341 tests) stays
green with it applied, so nothing currently depends on the lexical behaviour.
Worth considering alongside it: have `replace_one_file_with_symlink` refuse a computed
`link_target` that is not under `target_dir`, before deleting anything. That turns any
future instance of this class of bug into a clean refusal rather than a broken link, and
it is a cheaper invariant than keeping two path computations in step by inspection.
A regression test should cover the symlinked-root shape specifically; nothing in the
current suite exercises a path that reaches the inputdata tree through a symlink.
## Scope
- Reproduced identically on `main` and on the `fix-rimport-on-dir` branch. **Pre-existing;
not introduced by directory enumeration.**
- On the branch, the directory form reaches it too (`rimport /lnd`), because
enumeration resolves the named directory and then hands each discovered file to the same
relink path. Same single failure, same cause.
- Verified on Python 3.13.2. The mechanism is `os.path.relpath` versus `Path.resolve()`,
neither of which is version-sensitive, so the CI matrix (3.9–3.12) will behave the same.
## Recovery, if it has already happened
The staged copy is intact. Repoint the link by hand:
```bash
ln -sf "$STAGING/" "$INPUTDATA/"
```
Verified against the reproduction: the file is readable again afterwards, with the staged
copy as its target. Nothing needs to be re-staged.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.