upload-artifact icon indicating copy to clipboard operation
upload-artifact copied to clipboard

[bug] Symlinks are preserved by default

Open daskol opened this issue 1 year ago • 6 comments

What happened?

Transition from v4.3.4 to v4.3.5 broke regular building pipelines because actions/upload-artifact does not derefernce symlink anymore by default and upload symlinks as is but not target files.

See "Upload StarPU libraries" job for details.

  • Oringinal pipeline: https://github.com/nntile/nntile/actions/runs/10142088400/job/28040670213
  • Broken pipeline: https://github.com/nntile/nntile/actions/runs/10222093470/job/28286024223

What did you expect to happen?

  1. Pipelines are expected to work as before.
  2. Symlinks are dereferenced by default.
  3. This action providesd special option/flag for (de)referencing symlinks (kind of -L/-l in common POSIX utils).

How can we reproduce it?

You can fork repo or create pull request and trigger pipeline on push event.

Anything else we need to know?

No response

What version of the action are you using?

v4.3.5 (no issue with the previous v4.3.4)

What are your runner environments?

linux

Are you on GitHub Enterprise Server? If so, what version?

No response

daskol avatar Aug 02 '24 22:08 daskol

This also affected us. Thanks @daskol for reporting this issue.

erick-xanadu avatar Aug 05 '24 14:08 erick-xanadu

#589 Seems the same issue.

cbaeberle avatar Aug 06 '24 11:08 cbaeberle

Hi there! Riccardo from Meta, working in the React team and taking care of the React Native build pipeline on GH. This affected us as well.

cipolleschi avatar Aug 06 '24 12:08 cipolleschi

@robherley Could we revert the changes in the latest release 4.3.5 and create a new release 4.3.6? After that, we can continue working on this issue.

daskol avatar Aug 06 '24 13:08 daskol

Hey folks, we reverted & released a new version for v4.3.6 to address the regression.

robherley avatar Aug 06 '24 14:08 robherley

It would be really nice if this behaviour could be opt-in instead #93 #508.

Edit: after testing it seems that not all symlinks are preserved

sanjacob avatar Aug 15 '24 11:08 sanjacob

With v4.4.1, this is fixed:

  • https://github.com/actions/upload-artifact/pull/625
  • https://github.com/actions/upload-artifact/releases/tag/v4.4.1

robherley avatar Oct 07 '24 18:10 robherley

@robherley It seems that from the original issue steps 1. and 2. have been completed

What did you expect to happen?

  1. Pipelines are expected to work as before.
  2. Symlinks are dereferenced by default.
  3. This action providesd special option/flag for (de)referencing symlinks (kind of -L/-l in common POSIX utils).

Is there any timeline for delivery on step 3?

icfaust avatar Oct 08 '24 07:10 icfaust

My action builds a Linux library and uploads it to a tarball. The library consists of a libmylibrary.so.1.0 and a symlink libmylibrary.so that points to the former file. Since 4.4.1, that file is not included anymore.

teo-tsirpanis avatar Oct 08 '24 12:10 teo-tsirpanis

@icfaust I don't believe we're going to specifically work on (3) at this time, we were only focused on the regression

robherley avatar Oct 08 '24 13:10 robherley

@teo-tsirpanis Do you have an example or a minimum way to reproduce?

I've added a symlink test to upload-artifact's workflow, as well as toolkit too.

robherley avatar Oct 08 '24 13:10 robherley

@robherley

The job in question is Build-Native (ubuntu-latest, dev) and the symlink should be in lib/libtiledb.so in artifact tiledb-native-dev-linux-x86_64.

teo-tsirpanis avatar Oct 08 '24 14:10 teo-tsirpanis

@teo-tsirpanis is it a relative symlink? Could you ls -la the contents before it's uploaded?

Looks like this job is failing to lstat the file: https://github.com/TileDB-Inc/TileDB-CSharp/actions/runs/11227543765/job/31209939977#step:7:20

Warning: ENOENT warning during artifact zip creation. No such file or directory
Error: ENOENT: no such file or directory, lstat 'libtiledb.so.2.27'

The relative link might not be getting resolved 🤔

robherley avatar Oct 08 '24 14:10 robherley

We have the same issue as @teo-tsirpanis including the lstat warning. In our case, the symlink is created via CMakes set_target_properties, which seems to be good practice. See https://github.com/acados/qpOASES/pull/2

FreyJo avatar Oct 08 '24 15:10 FreyJo

It seems to be a relative symlink:

teo@theodore-tiledb:~/code/TileDB$ ls -la build/Default/tiledb/libtiledb.so
lrwxrwxrwx 1 teo teo 17 Sep 19 03:25 build/Default/tiledb/libtiledb.so -> libtiledb.so.2.27

teo-tsirpanis avatar Oct 08 '24 15:10 teo-tsirpanis

Okay, I'll look into a fix. For now feel free to pin to 4.4.0 if you are using relative symlinks.

robherley avatar Oct 08 '24 15:10 robherley

I made a PR for toolkit, I'll have my team take a look:

  • https://github.com/actions/toolkit/pull/1844

@teo-tsirpanis @FreyJo do y'all want to test with the changes? You can try uploading with my branch version:

uses: actions/upload-artifact@robherley/v4.4.2

robherley avatar Oct 08 '24 16:10 robherley

The workflow succeeded, thanks!

teo-tsirpanis avatar Oct 08 '24 18:10 teo-tsirpanis

v4.4.2 is out, v4 will be updated shortly!

robherley avatar Oct 08 '24 18:10 robherley

And v4 is released with the latest changes 🎉

robherley avatar Oct 08 '24 19:10 robherley

v4 has been temporarily rolled back to v4.4.1 due to:

  • https://github.com/actions/upload-artifact/issues/630

You can still use v4.4.2 for these symlink changes.

robherley avatar Oct 09 '24 01:10 robherley

Closing this out, we preserve both relative and absolute symlinks and have added integrations test to prevent a regression.

All versions >= v4.4.2 (including v4) have this behavior addressed.

robherley avatar Oct 16 '24 15:10 robherley