Skip to content

Commit

Permalink
[FIXED JENKINS-14780] ensure gradlew is executable
Browse files Browse the repository at this point in the history
  • Loading branch information
@ndeloof
ndeloof committed Aug 14, 2012
1 parent 2eb3ee8 commit 48d9a22
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions src/main/java/hudson/plugins/gradle/Gradle.java
View file
Expand Up @@ -141,6 +141,7 @@ private boolean performTask(boolean dryRun, AbstractBuild<?, ?> build, Launcher
if (useWrapper) {
String execName = (launcher.isUnix()) ? GradleInstallation.UNIX_GRADLE_WRAPPER_COMMAND : GradleInstallation.WINDOWS_GRADLE_WRAPPER_COMMAND;
FilePath gradleWrapperFile = new FilePath(build.getModuleRoot(), execName);
gradleWrapperFile.chmod(744);
args.add(gradleWrapperFile.getRemote());
} else {
args.add(launcher.isUnix() ? GradleInstallation.UNIX_GRADLE_COMMAND : GradleInstallation.WINDOWS_GRADLE_COMMAND);
Expand Down

16 comments on commit 48d9a22

@ikikko
Copy link
Member

@ikikko ikikko commented on 48d9a22 Aug 18, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is chmod argument '0744', not '744' ?

When I use gradle plugin Ver 1.18 included this code,
gradlew permission is broken after build execution and build error.
So, i clone source code and change to '0744', build becomes success.

@ndeloof
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chmod argument is an int, so 744 == 0744, isn't it ?

@wolfs
Copy link
Member

@wolfs wolfs commented on 48d9a22 Aug 19, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually in java a leading zero means octal. Therefore, 0744 == 508, isn't it?

@rbywater
Copy link
Member

@rbywater rbywater commented on 48d9a22 Aug 19, 2012 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ndeloof
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all SCM don't restore the executable bit, so running gradlew requires a pre-build step do chmod +x. The plugin handle this makes sense imho

@rbywater
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One other issue to consider - chmod'ing a file in Git counts as a change to the gradle wrapper file. Is this likely to cause an issue? Perhaps it could be changed to only chmod the file if it isn't already executable (as for instance in my case I changed the Git version of the wrapper to come out with the correct mask) and then only change the user bit rather than the rest of the mask?

@ikikko
Copy link
Member

@ikikko ikikko commented on 48d9a22 Aug 20, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think that I am as rbywater saying.

chmod need not be executed when SCM support to restore file permission (like Git) , and shouldn't.

@rbywater
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that I would support this as an option for those that require it (I'm guessing if your SCM doesn't retain the permissions mask that it won't care about it changing) but don't like the idea of doing it by default.

@ndeloof
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, will make this an option

@ndeloof
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed at a3dc7bc

@ikikko
Copy link
Member

@ikikko ikikko commented on 48d9a22 Aug 20, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i pull code and run test, and it's ok.

Thanks a lot!

@ndeloof
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks a lot for very constructive discussion here

@c089
Copy link

@c089 c089 commented on 48d9a22 Sep 10, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please publish a new release of the plugin with the patch from @ndeloof?

@evgeny-goldin
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd need to try out a new version as well: right now 1.18 sets ./gradlew permissions to "--wxr-x--T" making it unusable.

@ndeloof
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.19 released

@evgeny-goldin
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Not checking the checkbox (we have wrapper script already executable in the GitHub) solved my permissions issue. And checking it also works.

Please sign in to comment.