Skip to content

Fallback to plugin for Git data if no data found in builds#401

Closed
benjie332 wants to merge 2 commits intojenkinsci:masterfrom
ixaris:git-commit-update
Closed

Fallback to plugin for Git data if no data found in builds#401
benjie332 wants to merge 2 commits intojenkinsci:masterfrom
ixaris:git-commit-update

Conversation

@benjie332
Copy link

Updated the BuildUtil (and all processes that query builds) to fallback to using the plugin's data regarding Git commits/branches if no Git data is stored with the build. This can occur if the job does not use a Git SCM (such as a multi-job that triggers other jobs).

Otherwise, merge request updates with no change in commit, such as a title change, would be registered as unbuilt by the plugin and trigger a (useless) new build.

Added tests for merge request handler for the case in question, and also manually tested this for Jenkins version 2.7.2 and GitLab version 8.9.1.

Feedback (especially for new tests) is more than welcome! No test failures were introduced with the change.

…to using the plugin's data regarding Git commits/branches if no Git data is stored with the build. This can occur if the job does not use a Git SCM (such as a multi-job that triggers other jobs).
@coder-hugo
Copy link
Contributor

I think it would be even better to remove the whole "magical" code that relies on the data of the git-plugin. The CauseData should contain all relevant information for retrieving MR builds.

@benjie332
Copy link
Author

Fair enough. I tried that first, but found a few breaking tests so I decided to play it safe.

Question though, the build utils differentiates between merges..is that a workaround for Jenkins and the fact that you can try a merge before a build? If we go for CauseData that won't matter anymore

…e now made based on this plugin's own data

Refactored tests to trigger builds via plugin (as Jenkins based builds will no longer have the necessary data)
@benjie332
Copy link
Author

I dropped all Jenkins related logic from the BuildUtil class. I had to rework a few tests as Jenkins-triggered builds will no longer be considered by the plugin.

Not sure if this is a bit of a regression? It fits our use-case perfectly, but other configurations might suffer if we don't use the fallback approach.

@omehegan
Copy link
Member

@benjie332 are you still interested in working on this PR?

@benjie332
Copy link
Author

benjie332 commented Dec 14, 2016 via email

@HeikoBoettger
Copy link

@benjie332 @omehegan
Hi,
I actually forked from this branch and created my own pull request and removed all the dependencies as suggested in comment two on Sep 3 by coder hugo. To do so I added additions classes to store the commit-id, source and target branch of the triggering merge request. As well as the possibility to add a future feature to always build the head revision of the source branch. #431 is directly based on this branch and it seems not to have any conflicts since the last build. There is also interest from the GitLab site to get this merged. Since it generates some evil side effect such as triggering multiple builds for the same commit.

@benjie332
Copy link
Author

Ok I fully caught up now - as @HeikoBoettger mentioned, pull request #431 is essentially this pull request + some rebasing and conflict resolution. I have no problems with #431 being merged instead of this as they are essentially the same thing

@benjie332 benjie332 closed this Dec 18, 2016
@reuben-tonna-ixaris reuben-tonna-ixaris deleted the git-commit-update branch June 4, 2021 09:10
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.

4 participants