-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Consider tags when checking which branches to build #340
Conversation
I won't consider a behavioral change like this without one or more tests which show the misbehavior prior to the change and then show that the changed code fixes the tests. Adding a behavior without tests has the risk that the behavior could be lost or changed in a future version without being detected. Could you provide tests? |
Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests |
I'll see what I can come up with. Do you have any pointers on where to start? |
I've found that it is usually easiest for me if I start with my IDE (Netbeans) and have it generate a test skeleton for the class I'm changing (GitUtils in this case). Then I set a breakpoint on the area I'm about to change and run the tests of that class to see which tests already hit that breakpoint. Then I start refining the tests or adding more tests to cover the area. Usually the goal is to write a test which shows the failure I'm trying to fix. Once I have the failing test case, then I write the code to fix the failing test. Once I have a basic understanding of an area, then I apply my change, confirm the existing tests still run, set breakpoints in the modified code, then run the tests to see how it behaves with the existing tests. |
When using multiple branch specifiers, the git plugin uses an advanced selection mechanism as defined in `getAdvancedCandidateRevisons` which does not consider tags from the repository, only normal branches. E.g. when using the following Branch specifiers: - refs/remotes/origin/${B} - refs/tags/${B} where B is a build parameter having a default value of "master" never causes any tag to be build, regardless of what the user chooses. Include the tags to complete the list of valid branches to find the best matching ref for the current build.
This test checks that for multiple branch specifications including some tag references, the build succeeds and selects the correct revision.
d94ed3c
to
3da7adf
Compare
I have added a test case in a second commit. I could squash these commits if you want. |
I'd prefer the test remain a separate commit so that it can be evaluated first without your change (to watch it fail), then with your change. |
Makes sense to me. But some people prefer otherwise. Should I re-order the commits then? |
No need to reorder the commits. My evaluation is done on a forked repo and already needs me to perform some interactive steps while I'm evaluating a pull request. |
Hi. Any progress on this one? Caused me some grief because some other plugin required a new version and the whole release process broke because the wrong commit was checked out. |
Looks related to https://issues.jenkins-ci.org/browse/JENKINS-14917 and two other duplicate issues. I'm starting investigating this PR. |
I successfully tested this PR:
|
@MarkEWaite this pull request has conflicts. We need a slight update to make it current but I'd prefer not to ask an updated contribution as resolving the conflict is trivial and years have passed since this PR has been filed. Changes can be seen here https://github.com/jbq/git-plugin/tree/consider-tags-to-build I made sure tests are successful. May I push those changes to the jenkins repo? Or do you prefer doing it another way? |
Thanks for evaluating the pull request. If you're comfortable that it has not regressed or changed behavior in some other area, please merge your modified version and close the pull request. |
…uild When using multiple branch specifiers, the git plugin uses an advanced selection mechanism as defined in `getAdvancedCandidateRevisons` which does not consider tags from the repository, only normal branches. E.g. when using the following Branch specifiers: - refs/remotes/origin/${B} - refs/tags/${B} where B is a build parameter having a default value of "master" never causes any tag to be build, regardless of what the user chooses. Include the tags to complete the list of valid branches to find the best matching ref for the current build. Add testMultipleBranchesWithTags to GitSCMTest This test checks that for multiple branch specifications including some tag references, the build succeeds and selects the correct revision. Pull request #340
Merged manually |
Thanks. Can I get a copy of the integration test job that you created for it? I'd like to include it in my docker image (now in my docker-lfs repo) |
@MarkEWaite here it is: jbq/docker-lfs@f12b4b0 The Git repository URL may vary depending on your needs, I put my own fork to be able to push tags for tests. |
Thanks. I used your job definition as a basis for a pair of jobs
I've confirmed that second job fails as expected with the currently released plugin, and that it passes as expected with the tip of the git-plugin master branch. |
In one of the recent change to Git plugin behavior[1], tags are also search as part of branches. This cause projects that build on all branches to also build on all tags. This change allowing for tags to be ignored. The correct XML setting for this will need to be as follow. ``` <extensions> <hudson.plugins.git.extensions.impl.CloneOption> <shallow>false</shallow> <noTags>true</noTags> </hudson.plugins.git.extensions.impl.CloneOption> </extensions> ``` This is the generated result ``` <extensions> <hudson.plugins.git.extensions.impl.CloneOption> <shallow>false</shallow> <noTags>true</noTags> </hudson.plugins.git.extensions.impl.CloneOption> <hudson.plugins.git.extensions.impl.WipeWorkspace/> </extensions> ``` [1] jenkinsci/git-plugin#340 [2] jenkinsci/git-plugin@bfeda3e
In one of the recent change to Git plugin behavior[1], tags are also search as part of branches. This cause projects that build on all branches to also build on all tags. This change allowing for tags to be ignored. Default this option is false and missing from XML output. [1] jenkinsci/git-plugin#340 [2] jenkinsci/git-plugin@bfeda3e Change-Id: I2ed2290f9ef8ecd0d9e96aad1d7cbce2964bf2da
@jbq Was this bug fix actually merged in? Looking at the code, it seems unchanged, and with the latest version of Jenkins and the git-plugin, it doesn't seem like tags are triggering builds. |
It is included in git plugin 3.2.0 (released 28 Mar 2017) and git plugin 3.3.0 (released 20 Apr 2017). |
@MarkEWaite But why are the code changes suggested in this PR not in the master branch? This PR modifies |
Can you explain why you think that the changes in this PR are not in the master branch? My investigation shows that they are in the master branch. I confirmed with I looked for the specific test named in this test, and I looked for the specific lines from GitUtils and saw that they were added by Claudio Bley in a commit from Jul 2015. The link you referenced is to your fork of the repository. I suspect your fork of the repository is out of date compared to the GitUtils in the official repository. |
@MarkEWaite Ah, my mistake, I was accidentally looking at the master branch in @avdv's fork rather than the master branch of this repo. It looks like this code was merged after all. Despite that, creating a new Git tag does not seem to trigger a Jenkins build, as explained here. We're seeing this issue on two separate Jenkins deployments, both of which are using the 3.3.0 version of the git-plugin. Any suggestions on how to troubleshoot the issue? |
Unfortunately, I don't have any better suggestions than are offered earlier. The earlier comment says: Thanks. I used your job definition as a basis for a pair of jobs
I just checked with my local installation, and those steps no longer cause the build to trigger unless there is a commit applied to the userContent repo. The git polling is invoked, but it correctly detects that the SHA1 has already been built. That matches with what I would expect, since the git plugin considers a SHA1 hash that has been built as "done", whether that SHA1 is a branch or a tag. When I make a change inside the first job, then apply the tag, then the second job triggers as expected. As far as I can tell, this change is working as expected in the case that I had tested previously. |
@MarkEWaite Maybe I'm missing something, but what setup would you recommend if we do in fact want the behavior where the git plugin will trigger a new build on any new tag, regardless of whether the SHA1 has already been built? This is the behavior we have in cloud-based CI like CircleCI, and it's quite convenient. I think we can get this behavior by enabling the GitHub "push" setup, but many teams don't want to expose any aspect of their Jenkins to the public Internet. |
@josh-padnick I think you could parameterize the build and provide a parameter that is the name of the tag or the SHA1 that you want to build again. You could create a branch per tag and use Jenkins multi-branch pipeline to automatically create and destroy a job per branch. Using a single job to create different artifacts with potentially very different histories seems like it will tend to clutter the job history with unrelated changes as the job switches from one tag to the next. I've found (for me personally) that a job per branch has been a better working model, especially with the advent of Jenkins Pipeline. |
@MarkEWaite Thx for your prompt response. Hmm, I guess the issue I have is that I want the issuance of a git tag to automatically trigger a new build. Whether I'm using git polling or GitHub web hooks, when a new tag is added, the git plugin perceives no change and doesn't trigger the job at all. Any other suggestions on how I could get the git plugin to treat a new tag as a change and re-run the build? |
@josh-padnick, those two techniques are the only ones that I'd recommend. The plugin design doesn't view a tag name as adding additional meaning to a SHA1. I realize that your use model considers the tag name to be meaningful, and I can envision cases where a tag adds compelling meaning to a SHA1 (embed a product license into a build based on tag name, etc.). Unfortunately, that's not the way the git plugin perceives the world. Sorry. The plugin needs a separate commit before it will generate a new build (other than in the cases I described). That could be an empty commit, but it needs a separate commit. |
@MarkEWaite Thanks for the clarification. The philosophy of the Jenkins git plugin to focus only unique SHA1s stands in contrast to the approach by, say, CircleCI which kicks off a build in response to git commits, tags, Pull Requests, etc. It'd be great if the git docs were somehow clearer on this. For now, I just configured params on my Jenkins job to allow for deploying to a different environment, per your suggestion. Thx for being so responsive! |
Discussions have started about alternatives to allow the option of treating tags differently than they are currently treated. Good luck with your use! |
Project: openstack-infra/jenkins-job-builder 7b4b9dcb8979271fbc78272014bfc24402fcc47b Add `do-not-fetch-tags` to CloneOption for Git In one of the recent change to Git plugin behavior[1], tags are also search as part of branches. This cause projects that build on all branches to also build on all tags. This change allowing for tags to be ignored. Default this option is false and missing from XML output. [1] jenkinsci/git-plugin#340 [2] jenkinsci/git-plugin@bfeda3e Change-Id: I2ed2290f9ef8ecd0d9e96aad1d7cbce2964bf2da
In one of the recent change to Git plugin behavior[1], tags are also search as part of branches. This cause projects that build on all branches to also build on all tags. This change allowing for tags to be ignored. Default this option is false and missing from XML output. [1] jenkinsci/git-plugin#340 [2] jenkinsci/git-plugin@bfeda3e Change-Id: I2ed2290f9ef8ecd0d9e96aad1d7cbce2964bf2da
This change is really bad for projects with many tags that do not wish to poll on the tags. This should be limited to |
It works the other way around: specify |
But |
@orgads are you seeing a case of JENKINS-45447? If so, can you confirm that it accurately describes your case, and add more description if it does not accurately describe your case? I thought the extra tag rev-parse calls were from another change, rather than this one. |
Yes, this is it. I know that 3.1.0 is ok and 3.2.0 is bad. I didn't bisect, but this commit was the first suspect I've noticed. |
This needs to be rolled back. It is a pretty serious regression. Jenkins spends all its time parsing tags. We have plugins we can't use because we can't update the git plugin. We have a hundred or so builds. Each repository has dozens (if not hundreds) of tags. Also, no refspec I could find would stop the tags from getting parsed. |
A work in progress pull request #556 has been prepared which is exploring alternatives to allow this capability to remain in the plugin without the heavy performance impact on repositories with many tags. Refer to the last successful build if you'd like to assist with testing the current idea. |
Hi All, Tag based trigger is not working with multi branch project. Can anyone help me on this? |
@saravanan-devops I won't help you in the context of a GitHub pull request. That's not the place to request help. There are very, very few people that read GitHub pull requests. A request for help in a GitHub pull request spends plugin developer time which could be better used to benefit the entire community. Please ask your question in the Jenkins IRC channel or the Jenkins users mailing list. Even better, please use google search to find the existing references in the Jenkins users forum which describe the additional plugin which must be added in order to trigger builds based on tags. Those references will also explain some of the current limitations, and allow you to then assist other users with their questions. |
In one of the recent change to Git plugin behavior[1], tags are also search as part of branches. This cause projects that build on all branches to also build on all tags. This change allowing for tags to be ignored. The correct XML setting for this will need to be as follow. ``` <extensions> <hudson.plugins.git.extensions.impl.CloneOption> <shallow>false</shallow> <noTags>true</noTags> </hudson.plugins.git.extensions.impl.CloneOption> </extensions> ``` This is the generated result ``` <extensions> <hudson.plugins.git.extensions.impl.CloneOption> <shallow>false</shallow> <noTags>true</noTags> </hudson.plugins.git.extensions.impl.CloneOption> <hudson.plugins.git.extensions.impl.WipeWorkspace/> </extensions> ``` [1] jenkinsci/git-plugin#340 [2] jenkinsci/git-plugin@bfeda3e Change-Id: I2ed2290f9ef8ecd0d9e96aad1d7cbce2964bf2da
In one of the recent change to Git plugin behavior[1], tags are also search as part of branches. This cause projects that build on all branches to also build on all tags. This change allowing for tags to be ignored. The correct XML setting for this will need to be as follow. ``` <extensions> <hudson.plugins.git.extensions.impl.CloneOption> <shallow>false</shallow> <noTags>true</noTags> </hudson.plugins.git.extensions.impl.CloneOption> </extensions> ``` This is the generated result ``` <extensions> <hudson.plugins.git.extensions.impl.CloneOption> <shallow>false</shallow> <noTags>true</noTags> </hudson.plugins.git.extensions.impl.CloneOption> <hudson.plugins.git.extensions.impl.WipeWorkspace/> </extensions> ``` [1] jenkinsci/git-plugin#340 [2] jenkinsci/git-plugin@bfeda3e Change-Id: I2ed2290f9ef8ecd0d9e96aad1d7cbce2964bf2da
When using multiple branch specifiers, the git plugin uses an advanced
selection mechanism as defined in
getAdvancedCandidateRevisons
whichdoes not consider tags from the repository, only normal branches.
E.g. when using the following Branch specifiers:
where B is a build parameter having a default value of "master" never
causes any tag to be build, regardless of what the user chooses.
Include the tags to complete the list of valid branches to find the best
matching ref for the current build.