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.

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

IntrinsicError: File exists (os error 17) when attempting to overwrite a symlink in dist/ #18809

Closed
huonw opened this issue Apr 24, 2023 · 0 comments · Fixed by #18873
Closed
Assignees
Labels
Milestone

Comments

@huonw
Copy link
Contributor

huonw commented Apr 24, 2023

Describe the bug

If pants export-codegen ...'s is run on a target where the output includes a symlink, the second attempt to write that to dist/codegen fails with IntrinsicError: File exists (os error 17), potentially being unable to overwrite the symlink.

I'm guessing this applies to more than just export-codegen and shell_command, but that and shell_command are the easiest way I know to repeatedly attempt to write a symlink to a single path.

Reproducer:

cd $(mktemp -d)

cat > pants.toml <<EOF
[GLOBAL]
pants_version = "2.16.0rc1"
backend_packages = ["pants.backend.shell"]

[anonymous-telemetry]
enabled = false
EOF

cat > BUILD <<EOF
shell_command(
    name="symlink",
    command="ln -s whatever.txt link.txt",
    tools=["ln"],
    output_files=["link.txt"],
)

shell_command(
    name="normal",
    command="touch file.txt",
    tools=["touch"],
    output_files=["file.txt"],
)
EOF

pants export-codegen :symlink
# OK: ... Writing generated files to dist/codegen

pants export-codegen :symlink
# BUG: ... IntrinsicError: File exists (os error 17)

# delete the existing symlink, and try again:
rm dist/codegen/link.txt
pants export-codegen :symlink
# OK: ... Writing generated files to dist/codegen

# non-link works fine, for comparison:
pants export-codegen :normal
# OK: ... Writing generated files to dist/codegen

pants export-codegen :normal
# OK: ... Writing generated files to dist/codegen

All of the export-codegens work, except for the BUG one:

16:42:31.28 [INFO] Writing generated files to dist/codegen
16:42:31.28 [ERROR] 1 Exception encountered:

Engine traceback:
  in `export-codegen` goal

IntrinsicError: File exists (os error 17)

Pants version
2.16.0rc1

OS
macOS

Additional info
#17758 is mildly related, since both that and this are pants failing to correctly update the contents of dist/.

@huonw huonw added the bug label Apr 24, 2023
@huonw huonw added this to the 2.16.x milestone Apr 24, 2023
@huonw huonw changed the title IntrinsicError: File exists (os error 17) for second pants export-codegen ... including a symlink IntrinsicError: File exists (os error 17) when attempting to overwrite a symlink in dist/ Apr 24, 2023
@huonw huonw self-assigned this Apr 30, 2023
huonw added a commit that referenced this issue May 1, 2023
This applies a workaround that fixes #18809, for 2.16: before this PR,
repeated commands that write the exact same contents to `dist/` will
fail, if those contents include a symlink. After this patch, they will
succeed. For instance, `pants export-codegen ::` twice if any codegen
creates a symlink.

The particular problem of failing when re-materialising an entry only
surfaces with symlinks, because directories are created in "exists okay"
mode, and files are truncated if they already exist.

However, directories and files _do_ have problems when being
materialised over an entry of a different kind (#17758), but fixing that
seems like a broader issue, and likely too large to target 2.16 at this
point. After the change in this PR, we're at least back to the behaviour
in 2.15:

- directly rerunning commands that write to the workspace will always
succeed
- rerunning after changes may or may not (and, if it does, may or may
not give a valid result: #18849)

I've started on a potential fix for #17758 and #18849 in #18871, but, as
mentioned, it felt like it was getting too large and too "feature"-y to
land for 2.16. If/when a fix along those lines lands, this workaround
can likely be reverted.
huonw added a commit to huonw/pants that referenced this issue May 1, 2023
…8873)

This applies a workaround that fixes pantsbuild#18809, for 2.16: before this PR,
repeated commands that write the exact same contents to `dist/` will
fail, if those contents include a symlink. After this patch, they will
succeed. For instance, `pants export-codegen ::` twice if any codegen
creates a symlink.

The particular problem of failing when re-materialising an entry only
surfaces with symlinks, because directories are created in "exists okay"
mode, and files are truncated if they already exist.

However, directories and files _do_ have problems when being
materialised over an entry of a different kind (pantsbuild#17758), but fixing that
seems like a broader issue, and likely too large to target 2.16 at this
point. After the change in this PR, we're at least back to the behaviour
in 2.15:

- directly rerunning commands that write to the workspace will always
succeed
- rerunning after changes may or may not (and, if it does, may or may
not give a valid result: pantsbuild#18849)

I've started on a potential fix for pantsbuild#17758 and pantsbuild#18849 in pantsbuild#18871, but, as
mentioned, it felt like it was getting too large and too "feature"-y to
land for 2.16. If/when a fix along those lines lands, this workaround
can likely be reverted.
huonw added a commit that referenced this issue May 2, 2023
#18873) (#18878)

This applies a workaround that fixes #18809, for 2.16: before this PR,
repeated commands that write the exact same contents to `dist/` will
fail, if those contents include a symlink. After this patch, they will
succeed. For instance, `pants export-codegen ::` twice if any codegen
creates a symlink.

The particular problem of failing when re-materialising an entry only
surfaces with symlinks, because directories are created in "exists okay"
mode, and files are truncated if they already exist.

However, directories and files _do_ have problems when being
materialised over an entry of a different kind (#17758), but fixing that
seems like a broader issue, and likely too large to target 2.16 at this
point. After the change in this PR, we're at least back to the behaviour
in 2.15:

- directly rerunning commands that write to the workspace will always
succeed
- rerunning after changes may or may not (and, if it does, may or may
not give a valid result: #18849)

I've started on a potential fix for #17758 and #18849 in #18871, but, as
mentioned, it felt like it was getting too large and too "feature"-y to
land for 2.16. If/when a fix along those lines lands, this workaround
can likely be reverted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant