CircleCI-Public / python-orb

Common CircleCI tasks for the Python programming language.
https://circleci.com/developer/orbs/orb/circleci/python
MIT License
13 stars 37 forks source link

fix(cache): wrong check for symbolic link #89

Closed clementlecorre closed 2 years ago

clementlecorre commented 2 years ago

Problem at "Copy to cache directory" step

ln: failed to create hard link '/tmp/cci_pycache/lockfile': File exists

Exited with code exit status 1
CircleCI received exit code 1
circleci@ID:~/project$ ls -lah /tmp/cci_pycache/lockfile
lrwxrwxrwx 1 circleci circleci 14 Feb  9 14:23 /tmp/cci_pycache/lockfile -> ./Pipfile.lock
circleci@ID:~/project$ if [ -f "/tmp/cci_pycache/lockfile" ]; then echo "do unlink"; fi
circleci@ID:~/project$ ls -lah /tmp/cci_pycache/lockfile
lrwxrwxrwx 1 circleci circleci 14 Feb  9 14:23 /tmp/cci_pycache/lockfile -> ./Pipfile.lock
circleci@ID:~/project$ if [ -L "/tmp/cci_pycache/lockfile" ]; then echo "do unlink"; fi
do unlink

issue: https://github.com/CircleCI-Public/python-orb/pull/83#issuecomment-1033777125

Jaryt commented 2 years ago

Thanks for surfacing this! I do wonder why -f has been enough for me in my envrionments. I do agree that -L is the more appropriate test here.

clementlecorre commented 2 years ago

@clementlecorre Thank you! Tests passed. Could you verify this resolves your issue by targeting the dev:alpha version? (python@dev:alpha)

For the bug that I corrected here! it is good on my side, I just tested. ✅ 🎉

👀 I think there is a second problem with the checksum of the file. {{ checksum "/tmp/cci_pycache/lockfile" }} In my case here I think that the checksum function does not work on a symbolic link. https://github.com/CircleCI-Public/python-orb/blob/349f0de663ebd2deace6543e1f87e7f58094b6b7/src/commands/install-packages.yml#L114

error computing cache key: template: cacheKey:1:32: executing "cacheKey" at <checksum "/tmp/cci_pycache/lockfile">: error calling checksum: open /tmp/cci_pycache/lockfile: no such file or directory

You can do a second PR for this problem 👍

zigarn commented 2 years ago

BTW, the checksum of a symlink would really change if the content of the linked file changes? Maybe a hard link (if possible, but may not work on all executors) or a copy would be more accurate?

Jaryt commented 2 years ago

BTW, the checksum of a symlink would really change if the content of the linked file changes? Maybe a hard link (if possible, but may not work on all executors) or a copy would be more accurate?

Currently we are hardlinking, however, I think I would like to change this because of this issue here: https://github.com/CircleCI-Public/node-orb/issues/119

That's a good point though, and would be would be pretty easy to test. I'll get back to you with the results of that.

@clementlecorre Can you send me a link to your build please so I can see if there might be anything else interfering with this? It's strange that our integration tests are succeeding so I'm wondering if you may be hitting an edge case or that your environment may be different?

Jaryt commented 2 years ago
$ echo "abc123" >> out.txt
$ cat out.txt
abc123
$ ln out.txt test.txt
$ cat test.txt
abc123
$ md5sum out.txt
2c6c8ab6ba8b9c98a1939450eb4089ed  out.txt
$ md5sum test.txt
2c6c8ab6ba8b9c98a1939450eb4089ed  test.txt
$ unlink test.txt
$ md5sum test.txt
md5sum: test.txt: No such file or directory
$ md5sum out.txt
2c6c8ab6ba8b9c98a1939450eb4089ed  out.txt
$ ln -s out.txt test.txt
$ md5sum out.txt
2c6c8ab6ba8b9c98a1939450eb4089ed  out.txt
$ echo "abc123" >> out.txt
$ md5sum out.txt
df1e4ff2860cb602bf9a87a97c0174e2  out.txt
$ md5sum test.txt
df1e4ff2860cb602bf9a87a97c0174e2  test.txt

Here you can see that there are no differences between the sym and hard link as it follows the link to the source file, even if the contents of the file change.

clementlecorre commented 2 years ago

@Jaryt I just make a repo where I reproduce the problem. Repo: https://github.com/habx/circleci-python-orb-caching-issue Build: https://app.circleci.com/pipelines/github/habx/circleci-python-orb-caching-issue

I did another test on the checksum and I think it is not a bug of the function https://app.circleci.com/pipelines/github/habx/circleci-python-orb-caching-issue/2/workflows/0c6b52e5-6130-41e2-b39a-31ecb0bd7bad/jobs/4

Jaryt commented 2 years ago

@clementlecorre I found the source of the issue. This is occurring due to the app-dir parameter being default to .. This is correct, but the side effect is that something happens to the link (potentially at the checksum level) causing it to not find the file because it then searches at ./Pipfile.lock for example. Going to update the command to usereadlink -f` to fully evaluate the path, rather than using shortcuts.

Jaryt commented 2 years ago

Found another issue that was tangential to this that I initially over-looked. Going to close this out in favor of #90 Thanks for the assistance here! ❤️

clementlecorre commented 2 years ago

@Jaryt I just did a new test and I still have the problem with v2.0.1

INFO: Detected Package Manager pipenv
INFO: Cache directory already exists. Skipping...
INFO: Cache directory already exists. Skipping...
INFO: Linking /home/circleci/project/Pipfile.lock to /tmp/cci_pycache/lockfile
ln: failed to create symbolic link '/tmp/cci_pycache/lockfile': File exists

https://app.circleci.com/pipelines/github/habx/circleci-python-orb-caching-issue/5/workflows/6f034f5a-7970-4d20-af92-867f76700ecd/jobs/16

Jaryt commented 2 years ago

@clementlecorre Thanks for checking. Can you add cache-version: v2 as a parameter to the command? Hopefully that'll fix the issue with that file left over from the previous runs.

clementlecorre commented 2 years ago

@Jaryt Indeed it works with a caching reset! Thanks for your feedback!