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

[JENKINS-24467] fix and new test case #253

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
5 participants
@langers2k
Copy link

langers2k commented Sep 2, 2014

No description provided.

@cloudbees-pull-request-builder

This comment has been minimized.

Copy link

cloudbees-pull-request-builder commented Sep 2, 2014

plugins » git-plugin #472 SUCCESS
This pull request looks good

@jenkinsadmin

This comment has been minimized.

Copy link
Member

jenkinsadmin commented Sep 2, 2014

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

@daniel-beck

This comment has been minimized.

Copy link
Member

daniel-beck commented Sep 2, 2014

Passing PATH as parameter doesn't work?

@langers2k

This comment has been minimized.

Copy link
Author

langers2k commented Sep 2, 2014

It will as long as it's a ParameterActon, I could use something else to make it more clear.

@daniel-beck

This comment has been minimized.

Copy link
Member

daniel-beck commented Sep 6, 2014

What I mean is, it seems weird to distinguish between EnvironmentContributing and ParameterActions in this manner. It may work for your specific use case, but break numerous others.

@langers2k

This comment has been minimized.

Copy link
Author

langers2k commented Sep 8, 2014

It shouldn't break any use case from 2.2.4 or before as the code that's changing was there yet. Nor does it break the functionality needed to resolve JENKINS-22009 which is what the original code was put in to do. I fear the original fix for JENKINS-22009 was too broad as it only needed ParameterActions to resolve the issue so this is just refining that fix.

Is there a use case you're worried about? I'm happy to change the PR if it will help get it accepted.

@MarkEWaite

This comment has been minimized.

Copy link

MarkEWaite commented Sep 14, 2014

While evaluating this submission today (and last week), I saw an unexpected result which I am still investigating. My Debian Wheezy machines are running git version 1.7.10.4. That is the standard version for Debian Wheezy. On both Wheezy machines, the test fails before it ever reaches the assertion. It seems to fail because the git command is not able to find the git-upload-pack command.

I believe the difference is likely in the command line git program itself. I suspect that 1.7.10.4 required an external program "git-upload-pack" while later versions of git embedded the functions of that external program into the git executable, or changed the way they found that external executable. I intend to alter the test so that it will run on git 1.7.10.4, then merge the pull request into both the master branch and the 2.2.x branch.

Thanks for the pull request and for the time and energy you spent analyzing the root of the problem.

@MarkEWaite MarkEWaite closed this Sep 15, 2014

@MarkEWaite

This comment has been minimized.

Copy link

MarkEWaite commented Sep 15, 2014

The change has been included in the 2.2.x branch and in the master branch. Should be next available in the 2.2.6 release of the plugin, and also will be available when the 2.3 series of the plugin is released.

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