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

[FIXED JENKINS-37394] Add @Symbol annotations for step and tool. #35

Merged
merged 3 commits into from Sep 23, 2016

Conversation

@abayer
Copy link
Member

commented Aug 13, 2016

Specifically added them to the Descriptors.

cc @reviewbybees

Specifically added them to the Descriptors.
@reviewbybees

This comment has been minimized.

Copy link

commented Aug 13, 2016

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

@wolfs

This comment has been minimized.

Copy link
Member

commented Aug 14, 2016

It would be nice to have an integration test for that. Is this easily possible with the current JenkinsRule infrastructure?

@abayer

This comment has been minimized.

Copy link
Member Author

commented Aug 14, 2016

@@ -37,6 +37,8 @@ sourceCompatibility = '1.6'

dependencies {
compile 'org.jenkins-ci.lib:dry-run-lib:0.1'
compile 'org.jenkins-ci:symbol-annotation:1.3'
jenkinsPlugins 'org.jenkins-ci.plugins:structs:1.3@jar'

This comment has been minimized.

Copy link
@jglick

jglick Aug 15, 2016

Member

Uh, should suffice to add the structs dependency without specifying symbol-annotation, no?

This comment has been minimized.

Copy link
@abayer

abayer Aug 15, 2016

Author Member

Surprisingly, no! Gradle, man, I dunno.

This comment has been minimized.

Copy link
@jglick

jglick Aug 15, 2016

Member

Hmm. Just check the output *.hpi file carefully to make sure it

  • declared a plugin dependency on structs
  • does not include WEB-INF/lib/symbol-annotation.jar

This comment has been minimized.

Copy link
@abayer

abayer Aug 15, 2016

Author Member

All I know is I had compilation errors without it. shrug

This comment has been minimized.

Copy link
@wolfs

wolfs Aug 16, 2016

Member

Try using

jenkinsPlugins 'org.jenkins-ci.plugins:structs:1.3@jar' {
    transitive = true
}

When using @jar transitive is automatically set to false. You will now pull in transitive hpi files, if the plugins depends on other plugins. I think something to do this automatically should be incorporated in the gradle-jpi-plugin.

This comment has been minimized.

Copy link
@abayer

abayer Aug 16, 2016

Author Member

Hrm - changed that and it compiled, but then tests failed. Oy!

This comment has been minimized.

Copy link
@abayer

abayer Aug 16, 2016

Author Member

Fixed!

@jglick

This comment has been minimized.

Copy link
Member

commented Aug 15, 2016

🐝 assuming the artifact is correct despite the odd behavior of gradle-jpi-plugin or whatever it is called.

@abayer

This comment has been minimized.

Copy link
Member Author

commented Aug 16, 2016

Ok, what finally got both compilation and tests to work was a compileOnly dependency on symbol-annotation, a la Maven provided scope. Sigh.

@wolfs

This comment has been minimized.

Copy link
Member

commented Aug 16, 2016

At some point someone should revisit the gradle-jpi-plugin to fix this classpath mess. Could you give me a pointer to how this happens in Maven land?

@abayer

This comment has been minimized.

Copy link
Member Author

commented Aug 16, 2016

@jglick

This comment has been minimized.

Copy link
Member

commented Aug 16, 2016

No, for Maven the dependency is on the primary artifact, of type jar, but the plugin does some analysis of the dependency trail to decide which plugin dependencies to add to the manifest and which JARs to bundle.

@jglick

This comment has been minimized.

Copy link
Member

commented Aug 16, 2016

re🐝

@wolfs

This comment has been minimized.

Copy link
Member

commented Aug 18, 2016

What is the Cloudbees process for this PR? Should I merge it or are you going to?

@abayer

This comment has been minimized.

Copy link
Member Author

commented Aug 18, 2016

@abayer

This comment has been minimized.

Copy link
Member Author

commented Aug 18, 2016

@wolfs Up to you whether to merge it. =)

@reviewbybees

This comment has been minimized.

Copy link

commented Aug 18, 2016

This pull request has completed our internal processes and we now respectfully request the maintainers of this repository to consider our proposal contained within this pull request for merging.

@wolfs

This comment has been minimized.

Copy link
Member

commented Aug 19, 2016

@abayer With this change I still can't use Gradle in Pipeline since it is no SimpleBuildStep, right?

@abayer

This comment has been minimized.

Copy link
Member Author

commented Aug 19, 2016

@wolfs Correct. And more importantly, the gradle step isn't a durable task step, so it shouldn't be used in Pipeline even if it is changed to be a SimpleBuildStep.

@wolfs

This comment has been minimized.

Copy link
Member

commented Aug 20, 2016

@abayer That sound interesting. Would it be difficult to add a durable task step for the Gradle plugin? Is there a good example how to do that?
Regarding the features of durable task - can this also be used without using pipeline? E.g. I have Gradle running on a Slave, restart Jenkins, and then the Job reattaches and shows Gradle still running?

@abayer

This comment has been minimized.

Copy link
Member Author

commented Aug 22, 2016

@wolfs I'll defer to @jglick to explain what's possible in detail, but no, durable tasks only work that way within Pipeline jobs.

@jglick

This comment has been minimized.

Copy link
Member

commented Aug 31, 2016

Would it be difficult to add a durable task step for the Gradle plugin? Is there a good example how to do that?

Discussion better left to https://issues.jenkins-ci.org/browse/JENKINS-27393.

@wolfs wolfs merged commit eabffa3 into jenkinsci:master Sep 23, 2016
2 checks passed
2 checks passed
Jenkins This pull request looks good
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
@wolfs

This comment has been minimized.

Copy link
Member

commented Sep 25, 2016

@abayer I merged this but I do not know how to invoke these custom symbols from the Pipeline script. Could you give me some guidance? How would I invoke the Gradle build step? How would I use the gradle symbol on the ToolInstallation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
PRs
New
4 participants
You can’t perform that action at this time.