Skip to content

protect the master and maven-3.9.x branches to prevent accidental commits#1035

Open
elharo wants to merge 3 commits intomasterfrom
protect
Open

protect the master and maven-3.9.x branches to prevent accidental commits#1035
elharo wants to merge 3 commits intomasterfrom
protect

Conversation

@elharo
Copy link
Copy Markdown
Contributor

@elharo elharo commented Mar 5, 2023

The goal is to keep committers (i.e. me) from accidentally pushing straight into master. I think this does that, but I've only done this before through the github UI so please review carefully.

@michael-o
Copy link
Copy Markdown
Member

Well, this will also add overhead if you fix a typo and are safe to push directly.

@cstamas
Copy link
Copy Markdown
Member

cstamas commented Mar 5, 2023

And this prevents release plugin as well (when doing a release from master)?

Copy link
Copy Markdown
Member

@slawekjaranowski slawekjaranowski left a comment

Choose a reason for hiding this comment

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

Although I like PR review I wouldn't to do such strict restrictions.

  • all PR will be must past one by one and each will be must rebased with the latest master before merge
  • all even simple fix (typo) will be must pass by whole machine
  • we don't have a consesus for RTC
  • release from master will be not possible

@hboutemy
Copy link
Copy Markdown
Member

hboutemy commented Mar 5, 2023

And this prevents release plugin as well (when doing a release from master)?

this one would be a blocker to me

@olamy
Copy link
Copy Markdown
Member

olamy commented Mar 5, 2023

And this prevents release plugin as well (when doing a release from master)?

this one would be a blocker to me

just saying nothing really prevents. create a branch release/4.0.0-alpha or release/3.9.2 or release/3.8.10 when starting the release process and then release from this branch. As is we do not block people merging to main branches during release process...
Then once the release is done just create a PR from the release/xxx branch.
The current way of releasing is a bit of legacy from svn days,

@michael-o
Copy link
Copy Markdown
Member

And this prevents release plugin as well (when doing a release from master)?

this one would be a blocker to me

just saying nothing really prevents. create a branch release/4.0.0-alpha or release/3.9.2 or release/3.8.10 when starting the release process and then release from this branch. As is we do not block people merging to main branches during release process... Then once the release is done just create a PR from the release/xxx branch. The current way of releasing is a bit of legacy from svn days,

Bah, even more work...

@olamy
Copy link
Copy Markdown
Member

olamy commented Mar 5, 2023

And this prevents release plugin as well (when doing a release from master)?

this one would be a blocker to me

just saying nothing really prevents. create a branch release/4.0.0-alpha or release/3.9.2 or release/3.8.10 when starting the release process and then release from this branch. As is we do not block people merging to main branches during release process... Then once the release is done just create a PR from the release/xxx branch. The current way of releasing is a bit of legacy from svn days,

Bah, even more work...

says the one is asking people to create jira for everything :P
actually it's just 2 command line:

  • git checkout -b release/....
  • when done with the release from from the branch: gh pr create -B

I just find the "hey guys hold on your merge for a week or two I'm releasing so nobody touch anything" a bit archaic svn/cvs style :)
But this was just a comment from the peanut gallery. And nothing prevent someone to do it at the end of day.

BTW I'm not for ban direct push but not for this release reason.

@michael-o
Copy link
Copy Markdown
Member

And this prevents release plugin as well (when doing a release from master)?

this one would be a blocker to me

just saying nothing really prevents. create a branch release/4.0.0-alpha or release/3.9.2 or release/3.8.10 when starting the release process and then release from this branch. As is we do not block people merging to main branches during release process... Then once the release is done just create a PR from the release/xxx branch. The current way of releasing is a bit of legacy from svn days,

Bah, even more work...

says the one is asking people to create jira for everything :P actually it's just 2 command line:

* git checkout -b release/....

* when done with the release from from the branch: `gh pr create -B`

I just find the "hey guys hold on your merge for a week or two I'm releasing so nobody touch anything" a bit archaic svn/cvs style :) But this was just a comment from the peanut gallery. And nothing prevent someone to do it at the end of day.

BTW I'm not for ban direct push but not for this release reason.

...and how do you want to reconcile two diverging branches back to a linear history?

@olamy
Copy link
Copy Markdown
Member

olamy commented Mar 5, 2023

And this prevents release plugin as well (when doing a release from master)?

this one would be a blocker to me

just saying nothing really prevents. create a branch release/4.0.0-alpha or release/3.9.2 or release/3.8.10 when starting the release process and then release from this branch. As is we do not block people merging to main branches during release process... Then once the release is done just create a PR from the release/xxx branch. The current way of releasing is a bit of legacy from svn days,

Bah, even more work...

says the one is asking people to create jira for everything :P actually it's just 2 command line:

* git checkout -b release/....

* when done with the release from from the branch: `gh pr create -B`

I just find the "hey guys hold on your merge for a week or two I'm releasing so nobody touch anything" a bit archaic svn/cvs style :) But this was just a comment from the peanut gallery. And nothing prevent someone to do it at the end of day.
BTW I'm not for ban direct push but not for this release reason.

...and how do you want to reconcile two diverging branches back to a linear history?

I don't see the problem here. Can you explain?

You have maven-3.8.x ready for 3.8.25 release.
so create a branch release/3.8.25, then start the release process from this branch.
At the end, you can create a PR from release/3.8.25 to maven-3.8.x or just increment version in maven-3.8.x (as long as you have the tag, does the commit by the m-release-p really matter?)

@michael-o
Copy link
Copy Markdown
Member

michael-o commented Mar 5, 2023

And this prevents release plugin as well (when doing a release from master)?

this one would be a blocker to me

just saying nothing really prevents. create a branch release/4.0.0-alpha or release/3.9.2 or release/3.8.10 when starting the release process and then release from this branch. As is we do not block people merging to main branches during release process... Then once the release is done just create a PR from the release/xxx branch. The current way of releasing is a bit of legacy from svn days,

Bah, even more work...

says the one is asking people to create jira for everything :P actually it's just 2 command line:

* git checkout -b release/....

* when done with the release from from the branch: `gh pr create -B`

I just find the "hey guys hold on your merge for a week or two I'm releasing so nobody touch anything" a bit archaic svn/cvs style :) But this was just a comment from the peanut gallery. And nothing prevent someone to do it at the end of day.
BTW I'm not for ban direct push but not for this release reason.

...and how do you want to reconcile two diverging branches back to a linear history?

I don't see the problem here. Can you explain?

You have maven-3.8.x ready for 3.8.25 release. so create a branch release/3.8.25, then start the release process from this branch. At the end, you can create a PR from release/3.8.25 to maven-3.8.x or just increment version in maven-3.8.x (as long as you have the tag, does the commit by the m-release-p really matter?)

What if master has developed from then? A ff-merge isn't possible anymore, no?

@olamy
Copy link
Copy Markdown
Member

olamy commented Mar 5, 2023

I don't see the problem here. Can you explain?
You have maven-3.8.x ready for 3.8.25 release. so create a branch release/3.8.25, then start the release process from this branch. At the end, you can create a PR from release/3.8.25 to maven-3.8.x or just increment version in maven-3.8.x (as long as you have the tag, does the commit by the m-release-p really matter?)

What if master has developed from then? A ff-merge isn't possible anymore, no?

and so what is the problem?
the tag is there. it's the most important of a release. then just bump version in the main branch.

@elharo
Copy link
Copy Markdown
Contributor Author

elharo commented Jun 24, 2025

Bringing this one back up. Accidental and deliberate pushes to the master branch without review continue to happen and this is not good for security or reliability. I think @olamy has clearly indicated how the release process can work following this change.

@michael-o
Copy link
Copy Markdown
Member

Bringing this one back up. Accidental and deliberate pushes to the master branch without review continue to happen and this is not good for security or reliability. I think @olamy has clearly indicated how the release process can work following this change.

It should be obviously summarized because no one remembers. Also note that the "Merge Button" on GH does not replace a merge in the terminal especially because it cannot set the proper committer email address.

@elharo
Copy link
Copy Markdown
Contributor Author

elharo commented Jun 24, 2025

I think @olamy can explain better than me, but briefly the procedure would be to make a branch for the release, release from the branch, and then increment the version in master, manually if necessary.

@michael-o
Copy link
Copy Markdown
Member

I think @olamy can explain better than me, but briefly the procedure would be to make a branch for the release, release from the branch, and then increment the version in master, manually if necessary.

Overhead for a volunteer

@elharo
Copy link
Copy Markdown
Contributor Author

elharo commented Jun 24, 2025

Yes, it is extra overhead. IMHO it's worth it. The current situation is unacceptably vulnerable to supply chain attacks. Today any committer account can unilaterally install a crypto miner, backdoor, or other malware into Maven with a strong chance that it will go unnoticed before release.

@michael-o
Copy link
Copy Markdown
Member

Yes, it is extra overhead. IMHO it's worth it. The current situation is unacceptably vulnerable to supply chain attacks. Today any committer account can unilaterally install a crypto miner, backdoor, or other malware into Maven with a strong chance that it will go unnoticed before release.

Maven for the paranoid.

@elharo
Copy link
Copy Markdown
Contributor Author

elharo commented Jun 24, 2025

It's not like these problems haven't happened before multiple times in multiple projects. When people really are out to get you, paranoia is the rational state of mind.

@slawekjaranowski
Copy link
Copy Markdown
Member

Yes, it is extra overhead. IMHO it's worth it. The current situation is unacceptably vulnerable to supply chain attacks. Today any committer account can unilaterally install a crypto miner, backdoor, or other malware into Maven with a strong chance that it will go unnoticed before release.

even if we have a protected master (or other branches) I will do release by creating branch for release like release/x.x.x than add what I want to this branch and do a release ...

even more I can prepare own binary artifacts which will be released .... if nobody check reproducible build result ...

so protected branches don't protect us by supply chain attacks and so on

@elharo
Copy link
Copy Markdown
Contributor Author

elharo commented Jun 24, 2025

You might be able to do that. I and other non-PMC committers can't. And generally a release requires a review and a vote. Possibly that's a formality and you could skip the vote but the release would be noticed, and likely someone would take a closer look. And usually someone does check the reproducible build result.

OTOH you or I certainly could commit something to master with a reasonable likelihood no one would pay any attention. We can close that hole and we should. Let's not let the perfect be the enemy of the good.

@olamy
Copy link
Copy Markdown
Member

olamy commented Jun 24, 2025

I do not see this fixing any security problems...
The only potential problem fixed is that people merging to master while someone is building a release.
Well, on one side we want to have only PR workflow, but on the other side we keep the option to push directly to the main branch. Sounds a bit contradictory :)
Frankly, I don't have a strong opinion. I'm happy to be still able to push directly to master and so not have useless changes as PR, which will be in the release notes as maintenance.

@olamy
Copy link
Copy Markdown
Member

olamy commented Jun 24, 2025

But if doing so, maven-3.9.x must be protected too
The required approval must also be discussed, as it's not what we are used to doing.

@elharo
Copy link
Copy Markdown
Contributor Author

elharo commented Jun 24, 2025

To be clear, I want a PR only workflow, full stop.

I suppose if we were using some non-Github system, then there would be other non-PR mechanisms for guaranteeing code review, but two person sign-off on all changes is standard practice for good reason. Given that we're using github though, PRs are simply how that is done.

And yes, maven-3.9.x should be protected too.

@elharo elharo changed the title protect the master branch to prevent accidental commits protect the master and maven-3.9.x branches to prevent accidental commits Jun 24, 2025
@olamy
Copy link
Copy Markdown
Member

olamy commented Jun 24, 2025

To be clear, I want a PR only workflow, full stop.

Well such change can only come from what "we" want (maven dev community) and not from what "you" want (a single person opinion).
As far as I can see from this PR this doesn't look to reach some consensus.
So this can only be discussed and maybe voted on ML (as a democratic decision coming from the community)

I suppose if we were using some non-Github system, then there would be other non-PR mechanisms for guaranteeing code review, but two person sign-off on all changes is standard practice for good reason. Given that we're using github though, PRs are simply how that is done.

And yes, maven-3.9.x should be protected too.

@elharo
Copy link
Copy Markdown
Contributor Author

elharo commented Jun 25, 2025

I'm happy to discuss on the mailing list. I do think this is important to get right. I don't want Maven to to become the next cautionary tale on Hacker News.

@elharo
Copy link
Copy Markdown
Contributor Author

elharo commented Jun 25, 2025

Re consensus, https://maven.apache.org/developers/conventions/github.html states:

"We should always provide changes by Pull Request. Direct commits will be not visible in Milestones issues list nor Release Notes."

And I did not write the PR that put that language there. :-)

@olamy
Copy link
Copy Markdown
Member

olamy commented Jun 25, 2025

Not everything has to be in release notes. Some changes are pure cosmetic and doesn't affect users and turns to be useless noise in release notes

@olamy
Copy link
Copy Markdown
Member

olamy commented Jun 25, 2025

Re consensus, https://maven.apache.org/developers/conventions/github.html states:

"We should always provide changes by Pull Request. Direct commits will be not visible in Milestones issues list nor Release Notes."

And I did not write the PR that put that language there. :-)

I'm not an English native speaker, but I think there is a difference between should and must.

Re consensus, https://maven.apache.org/developers/conventions/github.html states:

"We should always provide changes by Pull Request. Direct commits will be not visible in Milestones issues list nor Release Notes."

And I did not write the PR that put that language there. :-)

https://maven.apache.org/developers/conventions/git.html

For committers
Committers may, of course, commit directly to the ASF repositories. For complex changes, you may find it valuable to make a pull request at github to make it easier to collaborate with others.

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