renovatebot / renovate

Home of the Renovate CLI: Cross-platform Dependency Automation by Mend.io
https://mend.io/renovate
GNU Affero General Public License v3.0
17.67k stars 2.33k forks source link

Support for Gerrit #4109

Closed derkoe closed 10 months ago

derkoe commented 5 years ago

What would you like Renovate to be able to do? It would be great to have support for Gerrit based Git repositories including the Gerrit change workflow.

Describe the solution you'd like Create a Gerrit change for each dependency update and also update this changes on every run. Hashtags may be used to label changes (like GitHub labels).

Describe alternatives you've considered Gerrit provides a REST API which could be used to query/modify changes.

Additional context Gerrit is still used in a lot of projects especially by Google (https://gerrit.googlesource.com/, https://fuchsia.googlesource.com/).

rarkins commented 5 years ago

Renovate uses branches extensively. Will this map to Gerrit's concepts?

derkoe commented 5 years ago

Yes - i'll map to a "change" in Gerrit. The only difference is that Gerrit only allows one commit in such a "change" so all changes after the first commit have to be amended and then pushed.

Maybe I'll find some time in the next weeks to provide a proof of concept implementation

rarkins commented 5 years ago

Renovate only every makes one commit off master and force pushes over the top if it needs to make a change. So far sounds compatible?

derkoe commented 5 years ago

Yes! Started with the implementation in my fork: https://github.com/derkoe/renovate/tree/feat/4109-gerrit

ambles commented 3 years ago

@derkoe how's your implementation so far? I wonder how does Google uses both Gerrit and Renovate together.

derkoe commented 3 years ago

Not very far (╯︵╰) - we are mostly using GitLab now, so I'm not sure if I'll ever finish that.

Does Google use Renovate with Gerrit? Where did you find that.

rarkins commented 3 years ago

I think Google's use of Renovate is limited to its open source repositories, where they use GitHub Issues

tprochazka commented 2 years ago

To create Gerrit PR it is only necessary to do simple push command git.exe push origin refs/heads/master:refs/for/master just instead of master there must by special reference refs/for/master

And you basically don't need to amend the last commit if you want just update the previous PR, you need just use the same Change-Id: I8deb93e6ffba6005234b0d46d5ae9f285f9775a8 inside of the commit message. So if you create a completely new commit with the same Change-id it will update the previous, of course only if it is still open.

avm99963 commented 2 years ago

I myself use Gerrit and would like to join in the efforts to make renovate available for gerrit, so I'll fork @derkoe's branch and try to figure out how to continue implementing it (I'm new to renovate!).

If someone else is also interested in continuing @derkoe's work, please say so and we can coordinate!

NiasSt90 commented 2 years ago

hi, I've started a prototype implementation. The changes can be found here it's still a proof-of-concept, therefore no tests and documentation...

the current state works (inclusive automerge/submit) with some drawbacks (see below). You need some special configuration entries (explained and shown at the bottom).

Warning: I'm new to renovate and still study the documentation and code to understand all of the details. Perhaps i'm misunderstood some details/features. Then correct me plz.

Renovate-with-Gerrit Workflow

Here are my (current) translation of the renovate-branch/pull-request-model to the gerrit-change-model:

  1. no branches, we map the branchname to a gerrit-topic or hashtag with: Config.js: branchPrefix: "master%topic=" to use of topics or branchPrefix: "master%t=" for hashtags , that's the most important part, it allows to identify existing gerrit-changes based on the branchname generated from renovate like master%topic=testing-library-react-13.x. This way we don't need branches on gerrit-side but still has a unique name to find an existing change based only on the branchname.
  2. install the gerrit-commit hook in initRepo to create a change-id if not already exists in commit-message.
  3. allow to set a remotePushPrefix, called from (gerrit)initPlatform with 'refs/for/'
  4. Config.js platformCommit: true to allow re-use of an existing change-id Renovate commitAndPush at first and then try to create a Pull-Request. For Gerrit the "Pull-Request" is a Change and was created automatically after pushing to special ref refs/for/$branch. Therefore the gerrit commitFiles function will allways try to search for an existing Change with the same branch AND topic/hashtag. If found it re-use the Change-Id in the commit message. This allows updating existing Gerrit-Changes.
  5. createPr() always search for an existing Gerrit-Change with Branch-and-Topic/Hashtag Important: because of first-commitAndPush and later call to createPr Gerrit has already created a Change for this commit. The Renovate-Branchname looks like master%topic=react-dom-18.x and could be split into branch/topic.
  6. for each pull-request/change we add the gerrit Code-Review label with +2 (if available) on our own to allow "automerge".
  7. getBranchStatus(branchNameWithTopic) will check if all the changes found with branchname and topic (should only be one) are submittable, this includes validation of Verified labels from CI or similar gerrit-specific submit rules,
  8. changed some refspecs in some git-commands to avoid syntax errors because of the generated branchname with %topic... inside. i.e. diff origin/$branchname changed to $branchname@{u}, i hope this has no effect on other platforms.

(current) Drawbacks

  1. where to place the pull-request body and title? currently i'm add the body as a message and check all previous messages for "same-content". Delete a message is not an option (admin-priviledge). The pull-request title is currently unused.
  2. no option to "ignore" a PullRequest (Gerrit-Change). For all non-automerge changes we need an option to allow "abandon" a PR. But a user/dev can't abandon a change from other users. Perhaps the user can down-votes (-2?) and later renovate calls "abandon"?

Unfinished stuff

Needed Renovate Config.js

I'm use the Gerrit-HTTP REST API with http-basic-auth (Prefix /a/). Therefore you need to generate a custom password for the gerrit-enovate-account.

image

These are the important part:

module.exports = {
  platform: 'gerrit',
  endpoint: '$GERRIT_HTTP_URL',
  username: "$USER",
  password: "$HTTP-PASSWORD",

  platformCommit: true, //needed to allow commit with an existing change-id
  branchPrefix: "master%topic=", //Important: don't use branches, we use Gerrit-Topics or HashTags  with %t=
  onboardingBranch: "master%topic=renovate-config", //Important: don't use branches, we use Gerrit-Topics
  gitNoVerify: ["push"], //Important: allow commit-hook

};
NiasSt90 commented 2 years ago

New revision (amend-commit) uploaded.

The (major) drawback, that for each renovate call a new and empty Gerrit-PatchSet was created is gone.

The (fake) branch-handling was improved, for each open gerrit-change a local branch was created (from initRepo) and registered with the commitSha.

All open gerrit-changes are automatically "updated" if origin/$basebranch was updated. This was done thru a special platform.commitFiles function.

NiasSt90 commented 2 years ago

New revision (amend-commit) uploaded.

In my opinion the proof of concept is done. After my vacation i would start to write the tests.

Any comments?

rarkins commented 2 years ago

Hi @NiasSt90, thanks for keeping us updated with your progress. I have never used Gerrit so have to admit some of it is a little 🤯 for me. Should we try to address the drawbacks first, e.g. things which might make this unworkable? Can you describe them in a bit more detail (not just what you tried, but what the high level requirements are) so that we can try to work out a solution? Also feel free to email me for an invite into our contributor-only Slack group

Shegox commented 2 years ago

@NiasSt90 I'm actually having some capacity as well and were wondering if you like support with implementing Gerrit support? Anything we can try/test/implement to help you?

mf75 commented 2 years ago

I don't know, whether nias (a friend of mine) will be able to answer himself, but let me just inform you, that he finished his vacation with some accident, he still struggles with. He will be all right and back on the keys soon. Stay tuned and wish him all the best.

leoluk commented 2 years ago

My company might be willing to fund this feature. Reach out to me if interested.

PhilipAbed commented 2 years ago

info about: getBranchStatusCheck/setBranchStatus, getRawFile, getJsonFile, deleteLabel, ensureCommentRemoval, getIssueList

branch status checks: are the status of the build+test+checks that you see in github, on every pull request in renovate, every platform has some sort of checks made before approving the pull request, use the platform API to get those. basic use is auto-merge ( some other side functionality), but the normal flows usually work without needing this.

getRawFile/getJsonFile: are a way to get the files from the repository, usually GetJsonFile files calls GetRawFile data, and converts it to json.

deleteLabel: is used for rebaseLabel , so we can rebase PRs based on a specific label being set.

ensureCommentRemoval: will have to call the repository's pull request/comment and make sure it is removed ( if necessary, these flows require specific comments to be made beforehand, like auto-merge comment)

getIssueList: is a way to get all issues from a specific repo, check the code see what it does exactly, could be open issues only in order to find the Dependency Dashboard, or Open a new Dependency Dashboard, read about it.

for any help needed, please ask.

Hope nias gets better.

sselberg commented 2 years ago

First and foremost, you should advertise this effort in the Gerrit community, I'm sure you will get a lot of valid input from there. Some nits from just reading through this thread.

magnusbaeck commented 2 years ago

One problem with the use of topics is that a change can have just a single topic, and users may want to use topics for other purposes (e.g. atomic cross-repo commits with the Submit Whole Topic feature).

NiasSt90 commented 2 years ago

hello, i'm back at work (for quite some time) and now i've restarted to work at this task. I've updated the above comment and branch with some new/changed features (hashtags).

First and foremost, you should advertise this effort in the Gerrit community, I'm sure you will get a lot of valid input from there. Some nits from just reading through this thread.

okay, i would do this soon.

  • I would advice against using topics, I think hashtags are more useful it you want to add metadata to the change you are creating to be able to query for it in Gerrit.

okay, i've modified my prototype to allow both, topics or hashtags (with %topic= or %t=). It works with hashtags too, but there was a small difference/problem. There could be more than one hashtag for each change. Currently i'm use the "first" hashtag to identify an existing renovate pull-request.

  • branchPrefix: "master%topic=" this indicates to me that you limit renovate to only work against the master branch (if this results in git push origin HEAD:refs/for/master%topic=$ID

yes, only a single branch is currently possible. I'm still searching for an idea how to change this.

Renovate use a local git repo for all platforms in the same way (hard coded git-commands), a specific platform can't break out of this workflow (currently). It creates branches and commits locally and push them afterwards. The pull-request will be created after the created branches were pushed. But for Gerrit the creation of a pull-request (change) is done automatically by the push refs/for/$branch...

that is not an option (or useful). Renovate needs a local copy of the repo to analyze it for possible dependency upgrades (that's the main work of renovate). The platform specific implementation part (see interface Platform in platform/types.ts) is rather small and there is much work with hard coded git-commands equal to all platforms.

NiasSt90 commented 2 years ago

New revision (amend-commit) uploaded.

It's now possible to ignore pull-requests by vote them with -2 (reject). If a change was rejected by a user then renovate will "abandon" this change on the next run.

anicke commented 2 years ago

Nice, I have tested the WIP and created a bunch of Gerrit reviews with updates and it works like a charm! I had to do some smaller things like wrap the project query parameter in a "encodeURIComponent" to get it working here (my project contains "/" chars). My fork can be found here. I also added some tests that you can copy.

sselberg commented 2 years ago
  • I would advice against using topics, I think hashtags are more useful it you want to add metadata to the change you are creating to be able to query for it in Gerrit.

okay, i've modified my prototype to allow both, topics or hashtags (with %topic= or %t=). It works with hashtags too, but there was a small difference/problem. There could be more than one hashtag for each change. Currently i'm use the "first" hashtag to identify an existing renovate pull-request.

Yes there may be more than one hashtag associated with a change, but isn't Renovate only interested in the hashtag that is set by renovate? And there should only be one of those so you should be able to just ignore the rest of the hashtags. I don't think I fully understand what you mean by problem. IMM You identify the change associated with a "Renovate-issue" by querying for the hashtag for that "Renovate-issue" (GET /changes/?q=hashtag:renovate-$ISSUE_ID) once you have identified it you don't care about the other hashtags.

  • branchPrefix: "master%topic=" this indicates to me that you limit renovate to only work against the master branch (if this results in git push origin HEAD:refs/for/master%topic=$ID

yes, only a single branch is currently possible. I'm still searching for an idea how to change this.

That's a quite hefty limitation ;-)

Renovate use a local git repo for all platforms in the same way (hard coded git-commands), a specific platform can't break out of this workflow (currently). It creates branches and commits locally and push them afterwards. The pull-request will be created after the created branches were pushed. But for Gerrit the creation of a pull-request (change) is done automatically by the push refs/for/$branch...

viceice commented 2 years ago

can't you create a PR manually on gerrit via an API call? that would make the implementation much easier.

magnusbaeck commented 2 years ago

can't you create a PR manually on gerrit via an API call? that would make the implementation much easier.

That's doable, see https://github.com/renovatebot/renovate/issues/4109#issuecomment-1217963289, but if a local copy of the repo is necessary anyway it's not clear that there's any point in creating the change (PR) itself via an API call. Once the Renovate patch exists locally it actually seems easier to just push the commit than to make multiple API calls to piece together a Gerrit change corresponding to the patch.

viceice commented 2 years ago

was just an idea to have a more easily workflow . don't like to go away from default renovate workflow to much

NiasSt90 commented 2 years ago

I've created a new prototype which follows the renovate branch-create/update/delete model more closely.

Now i'm allow the renovate user to create, update and delete references (branches). (new permissions needed)

It will create new branches for each dependency update like for the other platforms. Then it pushes these changes (directly) to gerrit. (no review/change creation at this time).

After this the createPr() call (create pull-request) will cherry-pick the top-commit of the created branch (which contains the dependency update) to the configured target branch (via Gerrit-Rest-API). At this time a Gerrit-Review (i.e. pull-request for renovate) will be created automatically. The original source-branch will be stored into a gerrit-hashtag, to find it later for getBranchPr/...

You can find these Gerrit-V2 Branch here.

I'm still need the platformCommit: true and gitNoVerify: ["push"] to allow the reuse of an existing change-id during update an existing pull-request. This way the second cherry-pick with same Change-Id will update the existing gerrit-change on target branch.

The benefits are:

The disadvantages are:

I personally would prefer this new variant, because it's more in line with the other platforms. The additional permissions to refs/heads/ (including force-push) is acceptable.

magnusbaeck commented 2 years ago

I suppose the dummy branch would be deleted when the change with the cherry-picked commit is abandoned or merged?

Would it be an option to push the commits to a namespace that isn't under refs/heads, e.g. refs/renovate? That way those branches won't be included in fetch operations.

From a user perspective I can't see that there are any benefits with this approach, but I get that it makes the implementation much easier.

viceice commented 2 years ago

renovate will delete all abandoned branches automatically, so no need for extra cleanup

NiasSt90 commented 2 years ago

I suppose the dummy branch would be deleted when the change with the cherry-picked commit is abandoned or merged?

yes, renovate deletes these branches automatically after the pull-request is submitted or rejected.

Would it be an option to push the commits to a namespace that isn't under refs/heads, e.g. refs/renovate? That way those branches won't be included in fetch operations.

perhaps this could be done with a special .git/config in the local renovate repository. i would try...

From a user perspective I can't see that there are any benefits with this approach, but I get that it makes the implementation much easier.

you don't need a special "branchPrefix" configuration and therefore you can use the baseBranches configuration.

sselberg commented 2 years ago

I totally understand why the "push it like it's GitHub" approach is tempting but FWIW, as a former Gerrit admin, I find it problematic for a number of reasons:

Apart from these objections there's the general rule of using a tool as it is intended to being used to consider. If you aren't you are bound to experience corner-cases, increase complexity and painting yourself into the proverbial corner down the line.

NiasSt90 commented 2 years ago

I totally understand why the "push it like it's GitHub" approach is tempting but FWIW, as a former Gerrit admin, I find it problematic for a number of reasons:

  • You are moving complexity from Renovate to the local gitconfig and (what is worse) the Gerrit ref ACLs. The latter makes the integration rather complex and fragile. Gerrit's ref ACLs are very powerful but also very complex.

the first is not true, the current v2 don't need any special local config. Only for hiding the branches behind a special ref like refs/renovate/* there is some more work in the local git config needed. (and local means the one that renovate creates himself on each run in a temporary directory, no user needs this config changes...)

The second is a valid argument, but in my opinion it's not that hard to create these rules/permissions for a single user. Often there exists already a user/group which can push/delete refs to allow merge of "hotfix" branches or similar. (i'm administer our company gerrit instance).

  • Creating a branch in git is cheap but handling the sum of all branches in the surrounding infrastructure can be very expensive. As a GitHub user you hardly ever notice this since the GitHub work-flow is based around creating and deleting a never-ending stream of branches in project and clones and the GitHub infrastructure is optimized to handle this. Gerrit's focus is on pre-merge code-review that doesn't require any extra, short-lived, branches. You only need long-lived branches like main and release branches and most Gerrit infrastructures are hence optimized for this.

You see performance issues from gerrit? In our company gerrit installation the CI runs only runs for "changes", not for direct-pushes to branches.

  • The pattern of pushing something to a short-lived branch while trying to submit some version of that commit that is also pushed for review is an error-prone corner-case for Gerrit. This has caused problems for us in the past when we were moving to a more Gerrit workflow from a PR/MR mindset. We have rooted out a number of bugs in this area but there are surely some left.

I don't see a problem in this workflow by cherry-pick a commit to another (the target) branch. Can you explain this in detail?

Apart from these objections there's the general rule of using a tool as it is intended to being used to consider. If you aren't you are bound to experience corner-cases, increase complexity and painting yourself into the proverbial corner down the line.

You argue now for v1 of the prototype? There was no requirements to gerrit-permissions (except that renovate is allowed to give +2 for the Code-Review label) but some general limitations in branch usage because of the "branchPrefix" limitation.

yes, only a single branch is currently possible. I'm still searching for an idea how to change this.

That's a quite hefty limitation ;-)

viceice commented 2 years ago

can't the V2 variant work with multiple branches?

NiasSt90 commented 2 years ago

can't the V2 variant work with multiple branches?

With V2 you can use the baseBranches: ["master", "next"], feature to update multiple branches. With V1 this is (still) not possible because of the branchPrefix hack.

sselberg commented 2 years ago

I absolutely think that you should aim for support of multiple branches. My point is that V2 is also a hack. The best way forward, to make a stable and reusable Gerrit integration, is to extract an abstract Adaptor type with GitHub, GitLab & Gerrit implementations, that "knows" how to upload a change-suggestion (PR, MR, Change) and submit/merge it, to separate these parts from the business logic.

NiasSt90 commented 2 years ago

I've uploaded a new version of V2 that hides all renovate branches behind refs/renovate. Perhaps i would allow to configure this as a platform-configuration option. Then you can configure refs/heads for visible renovate branches or refs/$other for invisible (to default fetch-spec) ones.

The documentation for the gerrit-platform now contains the needed permission snippet as an example.

I've seen that the github platform implementation use similar refs for some push operations (see pushCommitToRenovateRef) to avoid CI triggering. Perhaps this could be combined or generalized. But my knowledge to the github-platform-impl is rather small.

sselberg commented 2 years ago

I totally understand why the "push it like it's GitHub" approach is tempting but FWIW, as a former Gerrit admin, I find it problematic for a number of reasons:

  • You are moving complexity from Renovate to the local gitconfig and (what is worse) the Gerrit ref ACLs. The latter makes the integration rather complex and fragile. Gerrit's ref ACLs are very powerful but also very complex.

the first is not true, the current v2 don't need any special local config. Only for hiding the branches behind a special ref like refs/renovate/* there is some more work in the local git config needed. (and local means the one that renovate creates himself on each run in a temporary directory, no user needs this config changes...)

I consider having to use push.insteadOf in the ~/.gitconfig of the user that runs Renovate a hack.

The second is a valid argument, but in my opinion it's not that hard to create these rules/permissions for a single user. Often there exists already a user/group which can push/delete refs to allow merge of "hotfix" branches or similar. (i'm administer our company gerrit instance).

In the general case, on other Gerrit installations, you don't allow push/delete and you don't use "hotfix" branches. If you use Gerrit as it is intended you don't need push rights to anything except for refs/for/*, and since you only need long-lived branches you generally don't need delete rights either, typically these rights are given to the abstract "Project Owners" group which is a poor fit for a bot-account. If V2 is what get's merged I would give the general advice to everyone to not reuse an already existing general push-and-delete-refs group but set a strict namespace that the Renovate user can operate within and block push/submit to this namespace for all other users. Very few project admins would be aware of how Renovate works and what refs it needs push-rights to. The complexity comes from the fact that Gerrit's inherited ACLs makes it equally easy to block access for the Renovate user somewhere in the inheritance chain, since in the general case, pushing directly to branches isn't something you need to do when working with Gerrit.

  • Creating a branch in git is cheap but handling the sum of all branches in the surrounding infrastructure can be very expensive. As a GitHub user you hardly ever notice this since the GitHub work-flow is based around creating and deleting a never-ending stream of branches in project and clones and the GitHub infrastructure is optimized to handle this. Gerrit's focus is on pre-merge code-review that doesn't require any extra, short-lived, branches. You only need long-lived branches like main and release branches and most Gerrit infrastructures are hence optimized for this.

You see performance issues from gerrit? In our company gerrit installation the CI runs only runs for "changes", not for direct-pushes to branches.

And many companies have a more complex infrastructure surrounding Gerrit and the services in this infrastructure generally expects branches to be long-lived. Having branches that doesn't follow that contract is costly because every service that deals with branches must be aware of which these branches are so that they can be excluded.

  • The pattern of pushing something to a short-lived branch while trying to submit some version of that commit that is also pushed for review is an error-prone corner-case for Gerrit. This has caused problems for us in the past when we were moving to a more Gerrit workflow from a PR/MR mindset. We have rooted out a number of bugs in this area but there are surely some left.

I don't see a problem in this workflow by cherry-pick a commit to another (the target) branch. Can you explain this in detail?

This is an example of such a corner-case biting us, since developers back then still used a lot of short-lived branches. https://gerrit-review.googlesource.com/c/gerrit/+/90314

Apart from these objections there's the general rule of using a tool as it is intended to being used to consider. If you aren't you are bound to experience corner-cases, increase complexity and painting yourself into the proverbial corner down the line.

You argue now for v1 of the prototype?

No, I argue against V2.

There was no requirements to gerrit-permissions (except that renovate is allowed to give +2 for the Code-Review label) but some general limitations in branch usage because of the "branchPrefix" limitation.

yes, only a single branch is currently possible. I'm still searching for an idea how to change this.

That's a quite hefty limitation ;-)

reda-alaoui commented 2 years ago

In the general case, on other Gerrit installations, you don't allow push/delete and you don't use "hotfix" branches.

As another Gerrit admin, I concur with that statement.

NiasSt90 commented 2 years ago

I totally understand why the "push it like it's GitHub" approach is tempting but FWIW, as a former Gerrit admin, I find it problematic for a number of reasons:

  • You are moving complexity from Renovate to the local gitconfig and (what is worse) the Gerrit ref ACLs. The latter makes the integration rather complex and fragile. Gerrit's ref ACLs are very powerful but also very complex.

the first is not true, the current v2 don't need any special local config. Only for hiding the branches behind a special ref like refs/renovate/* there is some more work in the local git config needed. (and local means the one that renovate creates himself on each run in a temporary directory, no user needs this config changes...)

I consider having to use push.insteadOf in the ~/.gitconfig of the user that runs Renovate a hack.

looks like you completely misunderstand this. There is absolutely no requirements to a ~/.gitconfig of the renovate user account. I guess that most renovate users don't use a real account (with a $HOME) to run renovate. It's not necessary.

My latest V2 adds a second fetch-spec to the cloned repository config. This is similar to git clone -c "remote.origin.fetch=+refs/renovate/*:refs/remotes/origin/*" ....

This way i can use these refs more easily (like branches).

The second is a valid argument, but in my opinion it's not that hard to create these rules/permissions for a single user. Often there exists already a user/group which can push/delete refs to allow merge of "hotfix" branches or similar. (i'm administer our company gerrit instance).

In the general case, on other Gerrit installations, you don't allow push/delete and you don't use "hotfix" branches. If you use Gerrit as it is intended you don't need push rights to anything except for refs/for/*, and since you only need long-lived branches you generally don't need delete rights either, typically these rights are given to the abstract "Project Owners" group which is a poor fit for a bot-account.

That's not true in general. There are people using the git-flow workflow....not everyone can switch to continuous delivery...

If V2 is what get's merged I would give the general advice to everyone to not reuse an already existing general push-and-delete-refs group but set a strict namespace that the Renovate user can operate within and block push/submit to this namespace for all other users. Very few project admins would be aware of how Renovate works and what refs it needs push-rights to. The complexity comes from the fact that Gerrit's inherited ACLs makes it equally easy to block access for the Renovate user somewhere in the inheritance chain, since in the general case, pushing directly to branches isn't something you need to do when working with Gerrit.

feel free to add the needed documentation part.

  • Creating a branch in git is cheap but handling the sum of all branches in the surrounding infrastructure can be very expensive. As a GitHub user you hardly ever notice this since the GitHub work-flow is based around creating and deleting a never-ending stream of branches in project and clones and the GitHub infrastructure is optimized to handle this. Gerrit's focus is on pre-merge code-review that doesn't require any extra, short-lived, branches. You only need long-lived branches like main and release branches and most Gerrit infrastructures are hence optimized for this.

You see performance issues from gerrit? In our company gerrit installation the CI runs only runs for "changes", not for direct-pushes to branches.

And many companies have a more complex infrastructure surrounding Gerrit and the services in this infrastructure generally expects branches to be long-lived. Having branches that doesn't follow that contract is costly because every service that deals with branches must be aware of which these branches are so that they can be excluded.

they no longer are visible branches...they are hidden like refs/changes/*. This should solve this problem or?

  • The pattern of pushing something to a short-lived branch while trying to submit some version of that commit that is also pushed for review is an error-prone corner-case for Gerrit. This has caused problems for us in the past when we were moving to a more Gerrit workflow from a PR/MR mindset. We have rooted out a number of bugs in this area but there are surely some left.

I don't see a problem in this workflow by cherry-pick a commit to another (the target) branch. Can you explain this in detail?

This is an example of such a corner-case biting us, since developers back then still used a lot of short-lived branches. https://gerrit-review.googlesource.com/c/gerrit/+/90314

You argue with possible (or resolved) bugs in Gerrit against cherry-pick a change? Or i'm misunderstand something here? If you don't use merges between branches (not allowed), but you have multiple branches, how do you get code from one to another without cherry-pick? Manually?

Apart from these objections there's the general rule of using a tool as it is intended to being used to consider. If you aren't you are bound to experience corner-cases, increase complexity and painting yourself into the proverbial corner down the line.

You argue now for v1 of the prototype?

No, I argue against V2.

okay, but then currently i see no room for Gerrit support without a major refactoring in renovate. All of the git-cli commands (lib/util/git/*) needs to be abstracted to allow Gerrit enough room to work without any "hacks". But that's beyond my knowledge, i'm not a renovate core-dev and i'm new to this project, i'm still learning by studying the source. I've no knowledge about the other supported platforms.

I'm personally don't see the branch-model in Gerrit as an hack. With the branches inside a hidden ref that behavior is not visible outside (for users).

Take a look at the docs, the branch-model is currently a key-concept.

NiasSt90 commented 2 years ago

In the general case, on other Gerrit installations, you don't allow push/delete and you don't use "hotfix" branches. If you use Gerrit as it is intended you don't need push rights to anything except for refs/for/*, and since you only need long-lived branches you generally don't need delete rights either, typically these rights are given to the abstract "Project Owners" group which is a poor fit for a bot-account.

to allow auto-merge (recommended) the Code-Review label with max-score (+2) is needed. By default this is only allowed for "Project Owners".

But now for something different, there are two "problems" beyond the V1/V2/Workflow discussion.

  1. the pull-request body markdown syntax. Currently the pull-request body was stored as gerrit-comments. I see no other place to store it. But the syntax available there is not really documented? The current generated markdown looks quite ugly.

image

  1. Where to store renovate presets?
viceice commented 2 years ago

presets are normally also stored inside the git repo

NiasSt90 commented 2 years ago

presets are normally also stored inside the git repo

but Gerrit use project inheritance, therefore it could be handy to allow to store a config-preset in one of the parent projects?

If you take a look here the different platforms provide a special syntax/semantic for presets-urls.

sselberg commented 2 years ago

I totally understand why the "push it like it's GitHub" approach is tempting but FWIW, as a former Gerrit admin, I find it problematic for a number of reasons:

  • You are moving complexity from Renovate to the local gitconfig and (what is worse) the Gerrit ref ACLs. The latter makes the integration rather complex and fragile. Gerrit's ref ACLs are very powerful but also very complex.

the first is not true, the current v2 don't need any special local config. Only for hiding the branches behind a special ref like refs/renovate/* there is some more work in the local git config needed. (and local means the one that renovate creates himself on each run in a temporary directory, no user needs this config changes...)

I consider having to use push.insteadOf in the ~/.gitconfig of the user that runs Renovate a hack.

looks like you completely misunderstand this. There is absolutely no requirements to a ~/.gitconfig of the renovate user account. I guess that most renovate users don't use a real account (with a $HOME) to run renovate. It's not necessary.

My latest V2 adds a second fetch-spec to the cloned repository config. This is similar to git clone -c "remote.origin.fetch=+refs/renovate/*:refs/remotes/origin/*" ....

This way i can use these refs more easily (like branches).

The second is a valid argument, but in my opinion it's not that hard to create these rules/permissions for a single user. Often there exists already a user/group which can push/delete refs to allow merge of "hotfix" branches or similar. (i'm administer our company gerrit instance).

In the general case, on other Gerrit installations, you don't allow push/delete and you don't use "hotfix" branches. If you use Gerrit as it is intended you don't need push rights to anything except for refs/for/*, and since you only need long-lived branches you generally don't need delete rights either, typically these rights are given to the abstract "Project Owners" group which is a poor fit for a bot-account.

That's not true in general. There are people using the git-flow workflow....not everyone can switch to continuous delivery...

But it is true in the general case, a vast majority of Gerrit users globally doesn't use git-flow, they use Gerrit's own pre-submit code-review work-flow. If your using git-flow you don't need Gerrit.

For clarity: Using Gerrit-workflow is not the same as Continuous Delivery, and git-flow does not exclude Continuous Delivery.

If V2 is what get's merged I would give the general advice to everyone to not reuse an already existing general push-and-delete-refs group but set a strict namespace that the Renovate user can operate within and block push/submit to this namespace for all other users. Very few project admins would be aware of how Renovate works and what refs it needs push-rights to. The complexity comes from the fact that Gerrit's inherited ACLs makes it equally easy to block access for the Renovate user somewhere in the inheritance chain, since in the general case, pushing directly to branches isn't something you need to do when working with Gerrit.

feel free to add the needed documentation part.

With your latest version it wouldn't be necessary IIUC.

  • Creating a branch in git is cheap but handling the sum of all branches in the surrounding infrastructure can be very expensive. As a GitHub user you hardly ever notice this since the GitHub work-flow is based around creating and deleting a never-ending stream of branches in project and clones and the GitHub infrastructure is optimized to handle this. Gerrit's focus is on pre-merge code-review that doesn't require any extra, short-lived, branches. You only need long-lived branches like main and release branches and most Gerrit infrastructures are hence optimized for this.

You see performance issues from gerrit? In our company gerrit installation the CI runs only runs for "changes", not for direct-pushes to branches.

And many companies have a more complex infrastructure surrounding Gerrit and the services in this infrastructure generally expects branches to be long-lived. Having branches that doesn't follow that contract is costly because every service that deals with branches must be aware of which these branches are so that they can be excluded.

they no longer are visible branches...they are hidden like refs/changes/*. This should solve this problem or?

  • The pattern of pushing something to a short-lived branch while trying to submit some version of that commit that is also pushed for review is an error-prone corner-case for Gerrit. This has caused problems for us in the past when we were moving to a more Gerrit workflow from a PR/MR mindset. We have rooted out a number of bugs in this area but there are surely some left.

I don't see a problem in this workflow by cherry-pick a commit to another (the target) branch. Can you explain this in detail?

This is an example of such a corner-case biting us, since developers back then still used a lot of short-lived branches. https://gerrit-review.googlesource.com/c/gerrit/+/90314

You argue with possible (or resolved) bugs in Gerrit against cherry-pick a change? Or i'm misunderstand something here? If you don't use merges between branches (not allowed), but you have multiple branches, how do you get code from one to another without cherry-pick? Manually?

It's rather difficult to explain if you aren't at least somewhat familiar with the submit-logic in Gerrit. Could you buy the notion that if you are doing something that hardly no-one else does there's a higher risk of running into corner-cases that hasn't been discovered before? If you could also somewhat trust my expertise as a Gerrit developer and maintainer I'm trying to tell you that in this area (pushing the same commit to a short-lived branch and for review) such corner-cases live. That doesn't guarantee that Renovate will experience issues, but there have been a plethora of issues in this area previously simply because only a very small percentage of Gerrit installations do that. If you choose not to subscribe to this point of view that's OK too.

Apart from these objections there's the general rule of using a tool as it is intended to being used to consider. If you aren't you are bound to experience corner-cases, increase complexity and painting yourself into the proverbial corner down the line.

You argue now for v1 of the prototype?

No, I argue against V2.

okay, but then currently i see no room for Gerrit support without a major refactoring in renovate. All of the git-cli commands (lib/util/git/*) needs to be abstracted to allow Gerrit enough room to work without any "hacks". But that's beyond my knowledge, i'm not a renovate core-dev and i'm new to this project, i'm still learning by studying the source. I've no knowledge about the other supported platforms.

I can sympathize with that, and to the fact that in some sense it's better to have something imperfect than nothing.

I'm personally don't see the branch-model in Gerrit as an hack. With the branches inside a hidden ref that behavior is not visible outside (for users).

Take a look at the docs, the branch-model is currently a key-concept.

That's the docs for pull-requests, Gerrit doesn't operate with Pull Requests, Pull Requests are GitHub specific and that's were the "branch-model" comes from. Renovate seems to be hard-coded for GitHub (which is my whole premise) and this is the entire reason why you need to first push the result to a superfluous ref first before you can create the change-request.

P.s. I'm not trying to discourage this effort I'm simply trying to share from my past experience with Gerrit to help in making this integration so good that a multitude of Gerrit installations out there won't hesitate to use it. I don't feel that I'm very successful at doing so :-) so I'll leave it at that.

Thank you for contributing a Gerrit integration for Renovate!

NiasSt90 commented 2 years ago

That's not true in general. There are people using the git-flow workflow....not everyone can switch to continuous delivery...

But it is true in the general case, a vast majority of Gerrit users globally doesn't use git-flow, they use Gerrit's own pre-submit code-review work-flow. If your using git-flow you don't need Gerrit.

For clarity: Using Gerrit-workflow is not the same as Continuous Delivery, and git-flow does not exclude Continuous Delivery.

In my opinion Gerrit allows to enforce code-reviews, not more or less . I can (nearly) use every git capable workflow.

In my company there are projects that are using git-flow and there are projects with pre-submit code-review work-flow. All of them are enforcing code-reviews and that's the reason we choose Gerrit. But this becomes an academic discussion and we should stop here. ;)

You argue with possible (or resolved) bugs in Gerrit against cherry-pick a change? Or i'm misunderstand something here? If you don't use merges between branches (not allowed), but you have multiple branches, how do you get code from one to another without cherry-pick? Manually?

It's rather difficult to explain if you aren't at least somewhat familiar with the submit-logic in Gerrit. Could you buy the notion that if you are doing something that hardly no-one else does there's a higher risk of running into corner-cases that hasn't been discovered before? If you could also somewhat trust my expertise as a Gerrit developer and maintainer I'm trying to tell you that in this area (pushing the same commit to a short-lived branch and for review) such corner-cases live. That doesn't guarantee that Renovate will experience issues, but there have been a plethora of issues in this area previously simply because only a very small percentage of Gerrit installations do that. If you choose not to subscribe to this point of view that's OK too.

That's why i ask how you take changes from one-branch to another without merge or cherry-pick? In my company the (Gerrit) Cherry-Pick is used rather selden. I have no experience with Gerrit-Cherry-Picks. But i can't image difficulties by cherry-pick a single commit onto a branch....it's like apply a patchfile.

But:

(pushing the same commit to a short-lived branch and for review)

this is not true for my V2 workflow. At first a branch was created (contains the dependency commit) locally and pushed (to refs/renovate/$branch) and afterwards the pull-request (i.e. gerrit-cherry-pick) will be created against target branch.

The lifetime of the refs/renovate/$branch was always longer then the change/pr. But i see no dependency between the cherry-pick and the branch-commit after the cherry-pick was done.

In summary my V2 workflow uses two gerrit/git functions:

And you mean that: hardly no-one else does these two things?

I'm personally don't see the branch-model in Gerrit as an hack. With the branches inside a hidden ref that behavior is not visible outside (for users). Take a look at the docs, the branch-model is currently a key-concept.

That's the docs for pull-requests, Gerrit doesn't operate with Pull Requests, Pull Requests are GitHub specific and that's were the "branch-model" comes from. Renovate seems to be hard-coded for GitHub (which is my whole premise) and this is the entire reason why you need to first push the result to a superfluous ref first before you can create the change-request.

Renovate support more then the GitHub platform. Currently Gitlab, Bitbuck(Server), Azure, Gitea. And all of them can work with the pull-request model. But i would agree that it looks like "invented" for GitHub.

But from a developer perspective a pull-request is the same as a Gerrit-change. A Gerrit-change perhaps has more constraints like single-commit. But perhaps one of the other platforms can enforce this too?

sselberg commented 2 years ago

IIUC the model of:

  1. push to ref
  2. REST call And the creation of the extra "refs/renovate/..." ref, Is only necessary because there's a need to mimic the GitHub (et.al.) integration if you don't want to refactor Renovate.

If you can make renovate push to any ref, why you from just pushing to refs/for/refs/heads/$target and skip the REST call? That should be all you need to use Gerrit as it was intended and the end-result should be the same (bar the unnecessary "refs/renovate/..." refs).

NiasSt90 commented 2 years ago

IIUC the model of:

  1. push to ref
  2. REST call And the creation of the extra "refs/renovate/..." ref, Is only necessary because there's a need to mimic the GitHub (et.al.) integration if you don't want to refactor Renovate.

If you can make renovate push to any ref, why you from just pushing to refs/for/refs/heads/$target and skip the REST call? That should be all you need to use Gerrit as it was intended and the end-result should be the same (bar the unnecessary "refs/renovate/..." refs).

sadly no, at the time of the commitAndPush the target-branch (i.e. baseBranch) is not known (or better: not passed down to the platform.commitAndPush).

I will do some more investigations into this, perhaps there is a (clean) way to provide these information to the platform.commitAndPush. Perhap i'm simply add the "baseBranch" to CommitFilesConfig.

NiasSt90 commented 2 years ago

New Prototyp V3

You can find these Gerrit-V3 Branch here.

It's based on V1 and don't need special permission (except to allow +2 Code-Review label) for some refs. It works with multiple baseBranches and don't need a special branchPrefix. To get this to work i've added the baseBranch as parameter to commitAndPush(). I think that should be no problem, the information is always there (for each invocation). I've modified the utils/git-push method a little bit to allow to push custom-src/dst-refs.

This way it's possible to build a gerrit-push url like:

refs/for/${config.baseBranch}%t=sourceBranch-${config.branchName}

The original branchName was stored into an Gerrit hashtag (with prefix) to allow find existing PRs.

As in V1 from initRepo() all existing (renovate-) PRs were checked out as local branches (additional with origin/ prefix).

This way most util/git/* commands are work as expected or in detail:

sselberg commented 2 years ago

Awesome!

sselberg commented 2 years ago

Will config.branchName always contain "renovate"? It might be nice to explicitly see who created the HashTag if it doesn't. i,e "%t=Renovate-sourceBranch-$(config.branchName)

p.s. IIRC there are extension-points for hashtag creation validation so that once a set hash-tag pattern is established it wouldn't be to difficult to write a plugin that validates hashtag creation so that only the renovate-user (and f.i. admins) have the right to create hashtags with a certain pattern (f.i. "sourceBranch-renovate/.*").

NiasSt90 commented 2 years ago

Will config.branchName always contain "renovate"? It might be nice to explicitly see who created the HashTag if it doesn't. i,e "%t=Renovate-sourceBranch-$(config.branchName)

That depends on the branchName configuration. The default starts with "renovate/" ("branchPrefix"). But every user can change this like he wants. But in my opinion the defaults are good enough for Gerrit users.

p.s. IIRC there are extension-points for hashtag creation validation so that once a set hash-tag pattern is established it wouldn't be to difficult to write a plugin that validates hashtag creation so that only the renovate-user (and f.i. admins) have the right to create hashtags with a certain pattern (f.i. "sourceBranch-renovate/.*").

okay, to avoid/prevent potential abuse?

I hope the maximum length for a Gerrit hashtag is not that short, i couldn't found this in the documentation so far.