← Back to issue list

repo: address issue with fix_symlink() when pointed at directory

View original Github issue

Metadata

Project
snapcraft
Number
#3370
Type
pull request
State
merged
Author
cjp256
Labels
—
Created
Updated
Closed

Current evaluation

PR #3370 merged as 44d5277a7: fix_symlink() now warns about dangling symlinks pointing at directories instead of crashing shutil.copyfile (SNAPCRAFT-2AV), plus refactor/docs.

Resolution: closed resolved

Reason: The PR itself was merged to master as commit 44d5277a7 'repo: address issue with fix_symlink() when pointed at directory (#3370)', which replaced the failing _try_copy_local() copy attempt with a dangling-symlink warning and added documentation.

Evaluated by: mr-cal-bot

No scores available.

Issue body

repo: address issue with fix_symlink() when pointed at directory If target is a directory, simply warn about dangling symlink. Light refactoring to make it easier to follow and add documentation. Example: ``` name: test base: core18 version: '0.1' summary: Single-line description: | Bla Bla grade: stable confinement: strict parts: my-part: # See 'snapcraft plugins' source: stuff plugin: nil stage-packages: - openssl - jq override-build: | # Create some host files to use. mkdir -p /tmp/d1 touch /tmp/f1 ln -sf /tmp/d1 /tmp/sd1 ln -sf /tmp/f1 /tmp/sf1 # Symlink from part install to host files. ln -sf /tmp/d1 $SNAPCRAFT_PART_INSTALL/d1 ln -sf /tmp/f1 $SNAPCRAFT_PART_INSTALL/f1 ln -sf /tmp/sd1 $SNAPCRAFT_PART_INSTALL/sd1 ln -sf /tmp/sf1 $SNAPCRAFT_PART_INSTALL/sf1 ln -sf /tmp/does-not-exist $SNAPCRAFT_PART_INSTALL/does-not-exist # Similar for build dir. touch $SNAPCRAFT_PART_BUILD/file ln -sf $SNAPCRAFT_PART_BUILD/build-file $SNAPCRAFT_PART_INSTALL/build-file ``` Run: ``` $ mkdir stuff s snapcraft $ touch stuff/foo $ snapcraft ``` Previously, with the directories, snapcraft would fail: ``` File "/home/ubuntu/snapcraft/snapcraft/internal/repo/_deb.py", line 427, in unpack_stage_packages cls.normalize(str(install_path)) File "/home/ubuntu/snapcraft/snapcraft/internal/repo/_base.py", line 204, in normalize cls._fix_artifacts(unpackdir) File "/home/ubuntu/snapcraft/snapcraft/internal/repo/_base.py", line 252, in _fix_artifacts cls._fix_symlink(path, unpackdir, root) File "/home/ubuntu/snapcraft/snapcraft/internal/repo/_base.py", line 287, in _fix_symlink if not os.path.exists(target) and not _try_copy_local(path, target): File "/home/ubuntu/snapcraft/snapcraft/internal/repo/_base.py", line 311, in _try_copy_local shutil.copyfile(os.readlink(path), target) File "/usr/lib/python3.6/shutil.py", line 120, in copyfile with open(src, 'rb') as fsrc: ``` Now, the re-build will trigger the following messages: 'build-file' will be a dangling symlink 'does-not-exist' will be a dangling symlink 'd1' will be a dangling symlink 'sd1' will be a dangling symlink' The rebuild is required to activate teh fix_symlink() on the assets already in $SNAPCRAFT_PART_INSTALL. Snapcraft either needs to ensure that these checks are run only on installed files from the deb (the original intention), or as a complete pass on build/stage/prime to ensure consistency. This does not address that. Fixes SNAPCRAFT-2AV Signed-off-by: Chris Patterson <chris.patterson@canonical.com>

Evaluation history

Date Model Scores Action Summary
stealth/space-bunny-alpha — closed resolved PR #3370 merged as 44d5277a7: fix_symlink() now warns about dangling symlinks pointing at directories instead of crashing shutil.copyfile (SNAPCRAFT-2AV), plus refactor/docs.
stealth/space-bunny-alpha — closed resolved Merged in #3370 (44d5277a7): fix_symlink() now warns about dangling directory-target symlinks instead of crashing, with refactoring and documentation.
qwen/qwen3.6-35b-a3b — — Merged fix addressing fix_symlink() crashes when targeting directories. Updates behavior to warn about dangling symlinks, includes refactoring and documentation, and was approved by reviewers prior to merge.
qwen3.6-35b-a3b-mtp-q6 — — Merged fix for fix_symlink() directory target crash. Now warns about dangling symlinks instead of failing. Includes refactoring and docs. Reviewer requested additional test coverage for symlink paths.

Update history

No update history recorded yet.

Related work

  • Related To: snapcore/snapcraft#3497 (confidence 85%)

    Follow-up merged PR (405c57655) 'repo: fix regression in fix_symlink()' that further corrected relative/host-copy symlink handling and added spread tests.

  • Likely Fixed By: snapcore/snapcraft#3497 (confidence 60%)

    Later PR touching the same fix_symlink() logic, indicating residual regressions in the behaviour introduced here.

Related issues

Issue Project State Summary Similarity
#3497 repo: fix regression in fix_symlink() snapcraft merged Merged PR #3497 as 405c57655, fixing fix_symlink() to relink host-copied files correctly and adding spread tests for relative and host-copy cases.
80%
#478 Follow symlinks while hard-linking migrated files. snapcraft merged Merged PR #478 as commit 313023188, resolving symlinks with os.path.realpath() before hard-linking migrated files and preventing broken links in generated snaps.
70%