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

Workflow/pipeline support #10

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

qrsty
Copy link

@qrsty qrsty commented Jul 26, 2016

This PR adds support to run the unity3d plugin via pipeline scripts.

e.g.

node('unity') {
    def name = 'your_unity_install_name'
    def args = '-quit -batchmode -executeMethod MyClass.MyMethod'
    unity3d unity3dName: name, argLine: args
}

Contains support for the snippet generator:

custom step

generalized step

This required bumping up the required jenkins plugins dependency version to 1.642.3. This changed some unit tests to require MasterToSlaveCallable because of different security restrictions.

A dependency to org.jenkins-ci.plugins.workflow.workflow-step-api has been added.

This mainly adds the Unity3dBuilderStep class and changes Unity3dBuilder to implement the SimpleBuildStep interface. This changes the perform method to require a Run<?, ?> instead of an AbstractBuild<?, ?>. A Run does not have access to a workspace, so a FilePath is injected for alternative access to paths and Computers.

I've moved out some methods to a helper class for use by the Unity3dBuilderStep class.

The only issue I have left is a compiler warning that I can't seem to get rid of:
Warning unknown enum constant edu.umd.cs.findbugs.annotations.When.ANYTIME Unity3dBuilder.java C:/Repos/unity3d-plugin/src/main/java/org/jenkinsci/plugins/unity3d/Unity3dBuilder.java

It seems to be an issue with the com.google.guava dependency version being updated. This had to be raised due to the InjectedTest suite changing from the 1.642.3 jenkins plugins version.

Let me know if there's anything I should take a look at or improve.

qrsty added 4 commits July 18, 2016 16:18
…ped Jenkins core version dependency and refactored where required)
…es. Moved help files to webapp folder for common access.
throw AbortException in SimpleBuildStep perform to notify failed builds
@lacostej
Copy link

Cool. Thanks a lot for the PR. I'll look at it in a few days. I have to setup a 2.0 with workflow setup.

@arodus
Copy link

arodus commented Sep 15, 2016

Awesome work, finally there is a working pipeline support. But since your changes variables in the installation dir are not getting parsed anymore.

FATAL: The configured Unity3d installation directory (${PROGRAMFILES}\Unity5.4.0f3) is not recognized as a Unity3d home directory. Remember that the plugin adds per-platform suffixes and is searching for the executable at ${PROGRAMFILES}\Unity5.4.0f3\Editor\Unity.exe.

@qrsty
Copy link
Author

qrsty commented Sep 15, 2016

Where is ${PROGRAMFILES} defined? How are you specifying the string for the path? The Groovy script should be able to parse environment variables with double-quoted or slashy strings.

I'm not sure where to access build variables for pipeline jobs, as the build type is now a Run<?, ?> instead of an AbstractBuild<?, ?>. A Run does not have a getBuildVariables() method.

The offending code is likely here. Any fixes are appreciated.

@arodus
Copy link

arodus commented Sep 15, 2016

${PROGRAMFILES} is a default windows environment variable.
old

new

The upper picture is a screen from the original plugin, the lower with your modifications. The variables also do not get parsed in the tool configuration anymore. I will have a look on this issue maybe I can fix it.

@JiiimmyyN
Copy link

Have you had time to look into this yet?

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

looks good AFAICT

@wrjp
Copy link

wrjp commented Jul 15, 2018

Would actually be really neat to have to change in there ;)

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.

6 participants