Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[bug] Symlinks are preserved by default #590

Closed
daskol opened this issue Aug 2, 2024 · 22 comments
Closed

[bug] Symlinks are preserved by default #590

daskol opened this issue Aug 2, 2024 · 22 comments
Labels
bug Something isn't working

Comments

@daskol
Copy link

daskol commented Aug 2, 2024 •
edited
Loading

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.

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 daskol added the bug Something isn't working label Aug 2, 2024
daskol added a commit to nntile/nntile that referenced this issue Aug 3, 2024
@daskol
Pin down version of action/upload-artifact
c7f8b21 
Action `action/upload-artifact` with new release no longer dereferences
symlinks by default.

See actions/upload-artifact#590 for details.
@daskol daskol mentioned this issue Aug 3, 2024
Merged
daskol added a commit to nntile/nntile that referenced this issue Aug 3, 2024
@daskol
Pin down version of action/upload-artifact (#132)
ccaa40e 
Action `action/upload-artifact` with new release no longer dereferences
symlinks by default.

See actions/upload-artifact#590 for details.
@erick-xanadu
Copy link

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

@rmoyard rmoyard mentioned this issue Aug 5, 2024
Merged
@teo-tsirpanis teo-tsirpanis mentioned this issue Aug 5, 2024
Closed
@cbaeberle
Copy link

#589 Seems the same issue.

@cipolleschi
Copy link

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 cipolleschi mentioned this issue Aug 6, 2024
Closed
@daskol
Copy link
Author

daskol commented Aug 6, 2024

@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.

facebook-github-bot pushed a commit to facebook/react-native that referenced this issue Aug 6, 2024
@cipolleschi @facebook-github-bot
Try to restore caches for Hermes (#45908)
24e7f7d 
Summary:
We had CI on main failing consistently the past couple of days.
The problem is that the hermes pipeline is failing to create the iOS XCFramework with the error:
> unable to create a Mach-O from the binary at '/Users/runner/work/react-native/react-native/packages/react-native/sdks/hermes/destroot/Library/Frameworks/catalyst/hermes.framework/hermes'

The main cause is this upgrade of [upload-artifacts](actions/upload-artifact#590) which breaks symlinks.

The solution is to bump the caches and downgrade the `upload-artifact` actions.
## Changelog:
[Internal] - Try to fix CI for Hermes

Pull Request resolved: #45908

Test Plan: GHA must be green

Reviewed By: cortinico

Differential Revision: D60828616

Pulled By: cipolleschi

fbshipit-source-id: 6976b86dd67e2fd9d806ebaa62f47e39dc44b30d
@robherley robherley mentioned this issue Aug 6, 2024
Merged
@robherley
Copy link
Contributor

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

@fishilico fishilico mentioned this issue Aug 10, 2024
Closed
@sanjacob
Copy link

sanjacob commented Aug 15, 2024 •
edited
Loading

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

@robherley robherley mentioned this issue Sep 20, 2024
Merged
@robherley
Copy link
Contributor

With v4.4.1, this is fixed:

@icfaust
Copy link

icfaust commented Oct 8, 2024

@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?

@teo-tsirpanis
Copy link

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.

@robherley
Copy link
Contributor

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

@robherley
Copy link
Contributor

@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.

@teo-tsirpanis
Copy link

@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.

@FreyJo FreyJo mentioned this issue Oct 8, 2024
Merged
@robherley
Copy link
Contributor

@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 🤔

@Julow Julow mentioned this issue Oct 8, 2024
Merged
@FreyJo
Copy link

FreyJo commented Oct 8, 2024 •
edited
Loading

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 acados/qpOASES#2

@jdblischak jdblischak mentioned this issue Oct 8, 2024
Closed
@teo-tsirpanis
Copy link

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

@robherley
Copy link
Contributor

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

@robherley robherley mentioned this issue Oct 8, 2024
Merged
@robherley
Copy link
Contributor

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

@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 robherley mentioned this issue Oct 8, 2024
Merged
@teo-tsirpanis
Copy link

The workflow succeeded, thanks!

@robherley
Copy link
Contributor

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

@robherley
Copy link
Contributor

And v4 is released with the latest changes 🎉

@robherley robherley mentioned this issue Oct 8, 2024
Closed
@robherley
Copy link
Contributor

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

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

@robherley robherley mentioned this issue Oct 9, 2024
Closed
@iCodeSometime iCodeSometime mentioned this issue Oct 9, 2024
Closed
FreyJo added a commit to acados/acados that referenced this issue Oct 9, 2024
@FreyJo
Update qpOASES with cmake update (#1289)
643a1be 
Currently our CI is failing because the upload artifacts does not upload
the link files somehow.
This would be one fix.

Related issue that caused the breaking change:
actions/upload-artifact#590
@tkoskela tkoskela mentioned this issue Oct 9, 2024
Merged
@markfickett markfickett mentioned this issue Oct 9, 2024
Open
@robherley
Copy link
Contributor

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.

jdblischak added a commit to jdblischak/centralized-tiledb-nightlies that referenced this issue Oct 17, 2024
@jdblischak
Return to actions/upload-artifact@v4
48f88d5 
Revert "Use actions/[email protected] to fix artifact symlinks"

This reverts commit 82bd3e7.

xref: actions/upload-artifact#590 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@icfaust @daskol @cipolleschi @teo-tsirpanis @robherley @FreyJo @cbaeberle @sanjacob @erick-xanadu