-
Notifications
You must be signed in to change notification settings - Fork 22
Improve error message for broken symlinks on artifact_path. #23
base: main
Are you sure you want to change the base?
Conversation
Actually hold on, I'm not sure I know what I'm talking about.
|
Nah the issue is that we'd be walking from inside the container so the symlink will be broken. |
I thought that mark's example actually had a valid file in a valid location because https://github.com/MarkLodato/example-build/blob/main/.github/workflows/build.yaml#L17 still uploaded something? |
The symlink is valid. It's just not visible to the binary running in the container. I don't think this PR is sufficient to close #22. To improve the error message, we should (1) put a "known issue" on the main page and (2) if we detect that it's a broken symlink, mention that this is a known issue. Just saying "broken symlink" is still going to be confusing since it's not actually broken. |
I don't think this distinction is valuable. We're running this action in a container so we have no other view of the system. To us, the symlink is broken. We could provide more context around why the symlink may be broken but that's pretty much it.
I think it's a good idea to add it as a known issue. The symlink breakage is the result of the execution environment but it's broken to the code being run. I'll add more context to the error message but a broken symlink could be either broken by the fs mapping done by docker or broken on the system and we have no way of reliably differentiating between the two. |
Friendly reminder that this PR is still open :-). Probably good to submit even an imperfect solution now while we wait for a better one. |
I guess the alternative is: we could rewrite everything in typescript or something that wouldn't have the container sandbox. |
Fixes #22
Well, it doesn't really fix #22 but it's the best we can do to highlight the issue.