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

Incompatibility with 'application' plugin; startup scripts no longer executable #7

Open
cloudshiftchris opened this issue Sep 18, 2024 · 6 comments · May be fixed by #11
Open

Incompatibility with 'application' plugin; startup scripts no longer executable #7

cloudshiftchris opened this issue Sep 18, 2024 · 6 comments · May be fixed by #11
Labels
a:bug Something isn't working

Comments

@cloudshiftchris
Copy link

When applied with the application plugin the startup scripts (via distZip task) no longer have executable bit set.

@jjohannes
Copy link
Member

Thank you for opening the issue @cloudshiftchris.

@britter already brought this up, but we forgot to create an issue here.

@jjohannes jjohannes added the a:bug Something isn't working label Sep 20, 2024
@britter
Copy link
Member

britter commented Sep 20, 2024

Yeah, I think we can be more clever in preserving the execute permissions for some files. We could react to the presence of the application plugin and find the scripts it creates. We could also look for well known file extensions such as .sh or .bat. Finally we could inspect files to see whether they start with a shebang line.

@cloudshiftchris
Copy link
Author

I'd assumed (incorrectly) that CopySpec would use the permissions as 'defaults' if nothing more specific (i.e. child copy specs, etc) were provided.
Ideally we'd find a way to provide defaults such that we don't fall back to OS-inconsistent ones.

@jjohannes
Copy link
Member

For the start scripts, the permission is set to a fixed value by the application plugin:
https://github.com/gradle/gradle/blob/7f6e34676041df34961420edd3cf95e6b7b9f762/platforms/jvm/plugins-application/src/main/java/org/gradle/api/plugins/ApplicationPlugin.java#L229

Which means that these archive entries are already reproducible.

The best solution would be to not override this, but only set permissions for all other files (for which no default/convention exists).

@jjohannes
Copy link
Member

Need to do more research/testing, but maybe this is a solution (?): #11

@britter
Copy link
Member

britter commented Nov 22, 2024

Team discussion

This issue was discussed extensively during today's team meeting with @jjohannes, @ljacomet, and myself being present. Unfortunately at this point we can't present a solution, but I'd like to share what as been discussed so far:

  • @jjohannes did some experimenting by running a build on a Windows GitHub Actions runner and printing out file.getPermissions().toUnixNumeric(). This resulted in 644 being printed for files and 744 being printed for for directories. This raised the question if the problem that we're trying to fix with this plugin actually exists.
  • Next we reasoned about what exactly is setting file permissions when a new file is created. Does this depend on the operating system (version)? Or the filesystem? Is it inherited in some way? None of us knew, so we need to do some research here. If anybody reading this happens to know how it works and can provide a reference explaining it, feel free to comment below!
  • After that we moved on to look more closely at Gradle's FilePermissions API. There's a confusing bit in the JavaDoc that says "The default permissions used differ between files and directories and are as follows:" and then it lists 644 for files and 755 for directories. We did not fully understand what that is supposed to mean, given that the underlying file could have permissions different from this. As it turns out by looking at the documentation, these default seem to be used when configuring permissions and either file or dir permissions are not set: "Using empty configuration blocks for file or directory permissions still sets them explicitly, just to fixed default values.". So we also need to do some more research here to understand how exactly Gradle works here.
  • Last but not least, @ljacomet proposed to introduce a configuration flag for users to opt out of the plugin's behavior to set file permissions to 644, because that is what is breaking things when combined with the application plugin. We agreed to defer this until we have a better understanding of the issue and are sure that we can't fix it in a more general way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:bug Something isn't working
Projects
None yet
3 participants