Skip to content

[Support] Use all_read | all_write for createTemporaryFile (#83360) #8310

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

Merged
merged 1 commit into from
Mar 1, 2024

Conversation

JDevlieghere
Copy link

In a04879c, dsymutil switched from using TempFile to createTemporaryFile. This caused a regression because the two use different permissions:

  • TempFile opens the file as all_read | all_write
  • createTemporaryFile opens the file as owner_read | owner_write

The latter turns out to be problematic for dsymutil because it either promotes the temporary to a proper output file, or it would pass it to lipo to create a universal binary and lipo preserves the permissions of the input files. Either way, this caused issues when the build system was run as a different user than the one ingesting the resulting binaries.

I did some version control archeology and I couldn't find evidence that these permissions were chosen purposely. Both could be considered reasonable default.

This patch changes the permissions to all read | all write to make the two consistent and match the one currently used by the higher level abstraction (TempFile).

rdar://123722848
(cherry picked from commit 310ed33)

In a04879c, dsymutil switched from using TempFile to
createTemporaryFile. This caused a regression because the two use
different permissions:

 - TempFile opens the file as all_read | all_write
 - createTemporaryFile opens the file as owner_read | owner_write

The latter turns out to be problematic for dsymutil because it either
promotes the temporary to a proper output file, or it would pass it to
`lipo` to create a universal binary and `lipo` preserves the permissions
of the input files. Either way, this caused issues when the build system
was run as a different user than the one ingesting the resulting
binaries.

I did some version control archeology and I couldn't find evidence that
these permissions were chosen purposely. Both could be considered
reasonable default.

This patch changes the permissions to `all read | all write` to make the
two consistent and match the one currently used by the higher level
abstraction (TempFile).

rdar://123722848
(cherry picked from commit 310ed33)
@JDevlieghere
Copy link
Author

@swift-ci test

@JDevlieghere JDevlieghere merged commit 0a3a556 into stable/20230725 Mar 1, 2024
@JDevlieghere JDevlieghere deleted the jdevlieghere/rdar/123722848 branch March 1, 2024 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant