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

Only change file permission to default if no permission is configured #11

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jjohannes
Copy link
Member

Working theory:
The documentation of FilePermissions states:

The default permissions used differ between files and directories and are as follows:
[...] FILE: [...] (0644, rw-r--r--)

It's not very clear what that means, but it may mean that if you query for the permissions of a file, but no permission was configured, you get this default.

We could use that to check if a permission was potentially not set and only set it then. This will either:

  • Override an permission explicitly set to 644 with the same value (no-op)
  • Set the 644 for a undefined permission (this is what we want for reproducibility)
if (file.getPermissions().toString().equals("644")) {
  file.permissions(p -> p.unix("644"));
}

For files that were alreay configured explicitly, for example by the application plugin, we will do nothing.
This would fix #7.

@jjohannes jjohannes force-pushed the 7-refine-file-permission-setting branch from 9a1713f to 0e9ff25 Compare November 22, 2024 10:37
@britter
Copy link
Member

britter commented Nov 22, 2024

Okay so your theory is:

  • If the OS or file system created a file with permissions different from 644, Gradle would still report 644 if nobody else configured it.
  • In this case we explicitly configure 644 to make sure the file gets this permissions.
  • If getFilePermissions() returns something else, then somebody must have explicitly configured it in Gradle. In this case we don't touch it.

This would work if it's really the behavior of the file permissions API. I think we need to read the code to understand what's going on there. Also it looks like it will result in an ordering problem, because the application plugin could be applied after the reproducible builds plugin and the callback to set file permissions would be executed after the one registered by this plugin.

@jjohannes
Copy link
Member Author

This would work if it's really the behavior of the file permissions API. I think we need to read the code to understand what's going on there.

Yes. We have to do that and/or run some tests.

Also it looks like it will result in an ordering problem, because the application plugin could be applied after the reproducible builds plugin and the callback to set file permissions would be executed after the one registered by this plugin.

That should not be a problem, because then the application plugin will overwrite what reproducible-builds configured. If you apply application after reproducible-builds, it should work with the current release already.

@britter
Copy link
Member

britter commented Nov 22, 2024

That should not be a problem, because then the application plugin will overwrite what reproducible-builds configured. If you apply application after reproducible-builds, it should work with the current release already.

Oh yeah, that makes sense!

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.

Incompatibility with 'application' plugin; startup scripts no longer executable
2 participants