Skip to content

Variables handling refactoring#54

Merged
rpetti merged 22 commits intomasterfrom
variables_handling_refactoring
Jul 17, 2014
Merged

Variables handling refactoring#54
rpetti merged 22 commits intomasterfrom
variables_handling_refactoring

Conversation

@oleg-nenashev
Copy link
Member

stuartrowe and others added 6 commits May 2, 2014 17:15
…me so build parameters are properly evaluated
Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
I've also added substituteParameters(String, Node, AbstractProject, ...) to generalize non-build substitutions like checkouts.

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
Copy link
Member Author

Choose a reason for hiding this comment

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

Just a temporary solution to compile the code. I'll remove it and explicitly use MacroStringHelper.
The previous method was implementation private, hence there won't be any issues

@cloudbees-pull-request-builder

plugins » perforce-plugin #105 SUCCESS
This pull request looks good

Copy link
Member Author

Choose a reason for hiding this comment

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

It was the unused variable

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
@jenkinsadmin
Copy link
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

All methods have been migrated to the new MacroStringHelper API.

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
@cloudbees-pull-request-builder

plugins » perforce-plugin #107 SUCCESS
This pull request looks good

@oleg-nenashev
Copy link
Member Author

@rpetti
I'd like to convert the MacroStringHelper class to the internal-package state in order to get rid of the legacy API methods and to allow further modifications.

The change would slightly corrupt the backward compatibility. Since it's just an Util class, such change seems to be acceptable. What do you think about it?

@rpetti
Copy link
Member

rpetti commented May 23, 2014

I'm not sure how it would break any backwards compatibility, since it doesn't make use of any serialization methods, nor it is used by other plugins. I'd say go ahead and do what you feel is best.

@oleg-nenashev
Copy link
Member Author

It breaks the binary compatibility. If someone uses its public methods in dependent plugins or Groovy scripts, such change will lead to the crash. BTW, the probability of such usage is close to 0%

@rpetti
Copy link
Member

rpetti commented May 23, 2014

Yeah, nothing else should be using those Util classes, so it shouldn't break anything.

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
All auxiliary methods in MacroStringHelper have been converted to private.

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
Copy link
Member Author

Choose a reason for hiding this comment

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

Should be moved

@cloudbees-pull-request-builder

plugins » perforce-plugin #108 SUCCESS
This pull request looks good

…itution, etc

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
@cloudbees-pull-request-builder

plugins » perforce-plugin #111 SUCCESS
This pull request looks good

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
@cloudbees-pull-request-builder

plugins » perforce-plugin #114 SUCCESS
This pull request looks good

…es_handling_refactoring

Conflicts:
	src/main/java/hudson/plugins/perforce/PerforceSCM.java

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
@cloudbees-pull-request-builder

plugins » perforce-plugin #116 SUCCESS
This pull request looks good

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
…nvironment

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
@cloudbees-pull-request-builder

plugins » perforce-plugin #118 SUCCESS
This pull request looks good

Examples: (polling, workspace cleanup, etc)

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
@cloudbees-pull-request-builder

plugins » perforce-plugin #120 SUCCESS
This pull request looks good

It is a follow-up to the discussion of f62db63

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
@cloudbees-pull-request-builder

plugins » perforce-plugin #122 SUCCESS
This pull request looks good

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
@cloudbees-pull-request-builder

plugins » perforce-plugin #124 SUCCESS
This pull request looks good

@oleg-nenashev oleg-nenashev changed the title [IN_PROGRESS] - Variables handling refactoring Variables handling refactoring Jun 21, 2014
@oleg-nenashev
Copy link
Member Author

The request is ready for the final review

@krizleebear
Copy link

Thanks a lot for working on this issue! I've been waiting for a fix a long time. We're desperate for building different Perforce branches in Matrix builds!

Final question: When can we expect a new official version containing this fix? There was no release since January ...:/

@oleg-nenashev
Copy link
Member Author

@krizleebear
Have your tested changes on your installations?
I use it on my dev. installations, but any additional evaluation would be useful.

When can we expect a new official version containing this fix?

The current release approach presumes testing on @rpetti's development servers before the official release. Robert, would you like perform such testing? If no, I can use our infrastructure for the final testing.

@krizleebear
Copy link

@oleg-nenashev
I've got a testing server running, but this is still running with the official version of the plugin. I will try the 1.3.28 snapshot from https://jenkins.ci.cloudbees.com/job/plugins/job/perforce-plugin/124/org.jvnet.hudson.plugins$perforce/
Is this the right one?

@krizleebear
Copy link

I installed the .hpi in my testing environment. Unfortunately, I still get the error when configuring a BRANCH via configuration matrix:

hudson.plugins.perforce.utils.ParameterSubstitutionException: <//mib2_com/${BRANCH}/sqlite3/CMakeLists.txt //WRK/test.txt>: Found unresolved macro at '//mib2_com/${BRANCH}/sqlite3/CMakeLists.txt //WRK/test.txt'
at hudson.plugins.perforce.utils.MacroStringHelper.checkString(MacroStringHelper.java:159)
at hudson.plugins.perforce.utils.MacroStringHelper.substituteParameters(MacroStringHelper.java:145)
at hudson.plugins.perforce.utils.MacroStringHelper.substituteParameters(MacroStringHelper.java:83)
at hudson.plugins.perforce.PerforceSCM.getEffectiveProjectPath(PerforceSCM.java:661)
at hudson.plugins.perforce.PerforceSCM.checkout(PerforceSCM.java:864)
at hudson.model.AbstractProject.checkout(AbstractProject.java:1252)
at hudson.model.AbstractBuild$AbstractBuildExecution.defaultCheckout(AbstractBuild.java:624)
at jenkins.scm.SCMCheckoutStrategy.checkout(SCMCheckoutStrategy.java:86)
at hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:530)
at hudson.model.Run.execute(Run.java:1732)
at hudson.matrix.MatrixBuild.run(MatrixBuild.java:306)
at hudson.model.ResourceController.execute(ResourceController.java:88)
at hudson.model.Executor.run(Executor.java:234)
at hudson.model.OneOffExecutor.run(OneOffExecutor.java:43)

My matrix contains only one axis:

BRANCH=dev/work

@oleg-nenashev
Copy link
Member Author

Could you reference an issue on JIRA with such feature request?
That variables have not been substituted for the build, but it is possible to fix the issue on the project's level

@oleg-nenashev
Copy link
Member Author

@krizleebear
Please also mention what do you expect to get in Master Matrix Jobs. They do not have axis values, but also checkout the data

@krizleebear
Copy link

Aaaah, well. This explains everything! Thanks for this hint. I was trying this with every new version of perforce plugin and was disappointed every single time ;-)
I filed the issue https://issues.jenkins-ci.org/browse/JENKINS-23599 and assigned it to you. I also described the master checkout behaviour. I would even prefer to omit the initial checkout, but I guess that's a major change. So the Master Job just should take the very first value of the configuration axis.

BR
Christian

Behavior:
* MatrixConfiguration: Inject Values
* MatrixProject: Inject the first value or empty string if it is not available

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
@cloudbees-pull-request-builder

plugins » perforce-plugin #126 SUCCESS
This pull request looks good

@krizleebear
Copy link

I tested CloudBees build #126 and it works like a charm! Thanks for this extremely useful improvement! So now I'm really looking forward to the official release! :)

@krizleebear
Copy link

Is there any schedule when this change will be merged to the official branch and when this will result in an updated release?

rpetti added a commit that referenced this pull request Jul 17, 2014
@rpetti rpetti merged commit 2da2bf9 into master Jul 17, 2014
@krizleebear
Copy link

@rpetti Thank you a lot for merging this change to master branch! If you need support testing this version in order to release, feel free to contact me.

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