FS#45425 - [AUR4] Please set receive.denynonfastforwards=false (git config)

Attached to Project: AUR web interface
Opened by Masato Hashimoto (hashimo) - Tuesday, 23 June 2015, 06:56 GMT
Last edited by Lukas Fleischer (lfleischer) - Saturday, 27 June 2015, 12:14 GMT
Task Type Feature Request
Category Backend
Status Closed
Assigned To No-one
Architecture All
Severity Low
Priority Normal
Reported Version 4.0.0-rc3
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 4
Private No

Details

Currentry, git repos on aur4 is configured as "receive.denynonfastforwards=true".
It denies `push -f' but it's sometimes inconvenience and, (I think) unnecessary because most aur4 git repos are maintained by only one user.
This task depends upon

Closed by  Lukas Fleischer (lfleischer)
Saturday, 27 June 2015, 12:14 GMT
Reason for closing:  Won't implement
Additional comments about closing:  Non-fast-forwards cause all kinds of problems. Send an email to aur-support@archlinux.org if you have strong reasons for wanting to edit already pushed commits.
Comment by Lukas Fleischer (lfleischer) - Tuesday, 23 June 2015, 07:39 GMT
Actually, we do not set receive.denynonfastforwards at all. There is a check in the update hook that denies non-fast-forwards for regular users, though. The reason is that we want to prevent the history from being deleted. However, I personally also think that disallowing users to fix small mistakes in commits is a bad idea and I am open to suggestions on how to allow for amending commits while preventing users from erasing the commit log.
Comment by (Det) - Tuesday, 23 June 2015, 22:07 GMT
I went ahead and just asked this in StackOverflow: http://stackoverflow.com/q/31014227/1821548
Comment by Lukas Fleischer (lfleischer) - Wednesday, 24 June 2015, 11:07 GMT
You might have misunderstood me. I am not looking for a way to let Git judge whether a ref update is a "fix" or a "deletion" (because there isn't any sane way to do that without consulting a human and even then, there might be corner cases where the outcome is subjective). I am searching for a different concept. Having something like a publicly available reflog is one idea I had in mind.
Comment by Lukas Fleischer (lfleischer) - Wednesday, 24 June 2015, 12:17 GMT
Here is another idea: After a non-fast-forward fails, people can file a "non-fast-forward push request" by using the "File request" link from the package actions box. They insert the commit SHA1 of the new HEAD and a short explanation. A TU then decides whether to accept or reject the request, as usual. If it is accepted, the commit hash is added to a whitelist, such that the next non-fast-forward push succeeds.
Comment by (Det) - Wednesday, 24 June 2015, 12:30 GMT
So as I misunderstand it, the commit log can be erased by saying that you would just change the last message?
Comment by Lukas Fleischer (lfleischer) - Wednesday, 24 June 2015, 13:32 GMT
Assuming you are referring to my last comment: No, that cannot be done. The TUs would review the diff (and possibily the commit log, also) before accepting such a request.
Comment by (Det) - Wednesday, 24 June 2015, 14:51 GMT
All right, but why would the SHA1 then need to be inserted? Can't this be auto-detected?
Comment by Lukas Fleischer (lfleischer) - Wednesday, 24 June 2015, 15:11 GMT
No, it can't.
Comment by Levente Polyak (anthraxx) - Wednesday, 24 June 2015, 16:08 GMT
just out of curiosity what exactly does justify all this overhead just to be able to force push? If its about fixing a small bug, just use a new commit. I don't really see a gain here in having such complexity like filing requests for that (thats not KISS), also I don't see any need to force push something to "fix a small issue".
You should test your commits before pushing and not after pushing. IMHO its a bad idea to fix "small mistakes" that got pushed (and therefor published) by rewriting history, there is no point in masking mistakes in the history. How would that end if all open-source software f.e. on github would fix bugs by altering old commits and then force push? *scnr* :P
Comment by Lukas Fleischer (lfleischer) - Wednesday, 24 June 2015, 18:17 GMT
A fixup commit usually works but there are rare occassions where it doesn't, e.g. when a user wants to stay anonymous and accidentally committed his real name in the commit author info (happened before).
Comment by Dominik Fischer (XZS) - Wednesday, 24 June 2015, 22:21 GMT
I strongly support this proposal.


I often rewrite history when I notice faulty whitespace or other small errors in an older commit, the change far not enough to rectify a sensible commit message. "Fix typo" does not leave a very clean history to work with. Back-and-forth-commits often make it harder to find a certain line in the history. So allowing rewrites will actually make the history more expressive.

With it now necessary to contribute, AUR4 will see many users taking their first steps with git. Personally, when I learned git I rewrote an awful lot. So allowing rewrites keeps the histories clean from any embarrassing rookie mistakes that add no value.

Allowing such by request would predictably put much unneeded work on the TUs, as they would mostly have to review exactly these cases.

The dangerous potential of rewrites normally only unfolds when multiple people have to work with the same history. In the AUR, there always is only one commiter at a time, namely the package maintainer. Though this role may change, never will two people commit to the same repository. So allowing would not cause much harm here. However I acknowledge the fear of history deletion by an adopting user. So I would to propose the following routine for the update hook.


When an update comes in that is no fast-forward, examine all commits that would be deleted (i.e. the range old-master..new-master). Look up the author of these commits. If it is the user who is pushing (the current maintainer), allow the commit. If the author of any of these commits is someone else, reject the push. This would allow any maintainer to only change history he has written himself.

Still, he could update his package by always replacing a sole commit again and again. This could also be discouraged by only allowing rewrites if they leave the history with a given minimum number of commits.


Perhaps, some heuristic like this could replace the strict rule allowing fast-forwards only.
Comment by Levente Polyak (anthraxx) - Wednesday, 24 June 2015, 23:11 GMT
First at all, I agree on cryptocracks opinion: the privacy part because of leaked author info is valid and that should be somehow handled.

However all other reasons are just silly and actually just prove my point why this should not be done. Its just stupid to rewrite all history to fix things like typo and whitespace issues. Different users will have different reasons to "clean up the history" so it will or may end in totally weird rewrites that basically also may introduce breakage for commits that have been working before.

It causes harm, its not just about how many people are commiting to that repository, its also about the end-user point of view. For example I now simply have all repositories of AUR packages that I have installed checked out into a folder. Some scripts run through and check for new deltas upstream, if there are any for a package then I can review the changes and continue building. I store a logfile of commits per package for historical reasons. If someone fast-forwards in that repository then it breaks the reproducibility as I may not be able to re-build that certain point in the history (for whatever reason, I mean a rewritten history may also introduce regression). Its a nice "feature" that the history is static and whatever happens I can jump or use a commit where I know that it is working. If upstream has rewritten the history and I don't have any other checkout then its simply lost and is not reproducible anymore.
So its simply not true that the "dangerous potential of rewrites" only applies if multiple people work at the same repo, as mentioned having a reproducible history is a nice and important "side-effect". Its really a very bad habit rewriting history and should only be done in very very rare cases that justify it (f.e. like cryptocrack's example with the privacy info of a user), besides such corner-cases (where I also see TU interaction as mandatory) that not being able to rewrite has to be enforced.

A reproducible history and therefor reproducible source (PKGBUILD) input for building the package is a very high good that we should try to maintain as good as its possible.

TL;DR:
- maybe consider adding an easy way for users to request _corner case_ rewrite that are somehow approved by TU's (this has to include the history for review)
- never allow users to rewrite history by themselves under no circumstances
Comment by Evangelos Foutras (foutrelis) - Wednesday, 24 June 2015, 23:34 GMT
Rewriting history is a very inconsiderate thing to do in any public repo. The privacy point is also moot; any leaked information should automatically be considered compromised. There's no undo button once something is made public, no matter how short the exposure period was.
Comment by Lukas Fleischer (lfleischer) - Thursday, 25 June 2015, 05:28 GMT
The main problem with non-fast-forwards is that it breaks `git pull`. If you edit the history after users already pulled the faulty commits, end users would need to deal with that, e.g. by using git-reset(1). Never ever edit history in a public repo unless it is really needed.

It is also true that any leaked information should be considered compromised but we should still give the users a possibility to minimize the degree of damage. Ianal but we might even be forced to do that, e.g. by the right to be forgotten or by other laws governing data protection and privacy. We should make it clear that "made a typo" definitely isn't a valid reason for a non-fast-forward push, though.
Comment by hurricane pootis (HurricanePootis) - Monday, 17 April 2023, 04:36 GMT
Hey, has there been any progress on this? I think allowing authors to go back and remove leaked author info is super useful
Comment by Doug Newgard (Scimmia) - Monday, 17 April 2023, 05:02 GMT
That's already doable by going through a TU/PM. Allowing it for everyone is a terrible idea, force pushing to public repos should only be done in extreme circumstances.

As far as progress, "Reason for closing: Won't implement" is pretty much it.
Comment by hurricane pootis (HurricanePootis) - Monday, 17 April 2023, 22:22 GMT
Alright cool. Do I have to contact a TU/PM on the IRC for AUR devs?
Comment by Doug Newgard (Scimmia) - Monday, 17 April 2023, 23:25 GMT
You can do it via IRC (#archlinux-aur on libera) or via the mailing list (aur-general)

Loading...