Request: Changing commit message in llvm trunk

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

Request: Changing commit message in llvm trunk

Martell Malone
Hi

I'm trying to change a commit message that became malformed when pushing to trunk

svn propedit svn:log --revprop -r 243434
svn: E175002: DAV request failed; it's possible that the repository's pre-revprop-change hook either failed or is non-existent
svn: E175002: Repository has not been enabled to accept revision propchanges;
ask the administrator to create a pre-revprop-change hook

As described here commit messages are not versioned on svn
Would it be possible to get a hook made for this
http://stackoverflow.com/questions/692851/can-i-go-back-and-edit-comments-on-an-svn-checkin

It could be very useful for 3 reason.

1.
Currently if someone makes a commit and does not have it is not automatically closed by phabracator by adding
Differential Revision: http://reviews.llvm.org/DXXXXX
at the end of their commit
The committer then often has to go back and manually enter a message to tell someone that their patch has landed

With a hook for this it would be easy to update the message invoking phabracator to now close the revision as it has landed.

we could even quite easily create a script for this for any committer to use to speed up the process, example here is a script called landed

landed D11511 r243434

2.
If a patch is reverted at some point because it is broken.
It would be nice to be able to edit the commit message in rXXX to tell the reader that the commit was reverted in rYYY and reapplied with a fix in rZZZ

3.
If you have a pesky typo in your commit like I have :)

I'm sure the hook could be made so that only an author can edit their own commit message or an admin that can.

Kind Regards
Martell



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Request: Changing commit message in llvm trunk

Anton Korobeynikov-2
> It could be very useful for 3 reason.
This will surely break git mirrors.

--
With best regards, Anton Korobeynikov
Faculty of Mathematics and Mechanics, Saint Petersburg State University
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Request: Changing commit message in llvm trunk

Nick Lewycky
Anton Korobeynikov wrote:
>> It could be very useful for 3 reason.
> This will surely break git mirrors.
Sounds like the git mirror's problem.
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Request: Changing commit message in llvm trunk

Tim Northover-2
>> This will surely break git mirrors.
>
> Sounds like the git mirror's problem.

And every user of those mirrors, unless we have the resources to forge
a SHA1 hash for the new commit message.

Tim.
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Request: Changing commit message in llvm trunk

James Y Knight-2
In reply to this post by Nick Lewycky
On Jul 28, 2015, at 9:34 PM, Nick Lewycky <[hidden email]> wrote:
> Anton Korobeynikov wrote:
>>> It could be very useful for 3 reason.
>> This will surely break git mirrors.
> Sounds like the git mirror's problem.


I don't think the git mirror would be *terribly* broken, it'd just never see the commit message change. But, the annoying part would be if/when someone recreated the git mirror, as it would then contain the new commit message, and thus that commit and all subsequent commits would have a different commit-hash.

This will similarly "break" anyone's svn mirrors (e.g. done by svnsync), as it cannot automatically detect that a commit message has been updated. If you were paying attention, you could copy the new data via svnsync copy-revprops, but that basically requires manual interaction.

Independently of those particular breakages, it's just a terrible idea to allow unversioned modifications to be made to the version control's historical record. That's the kind of thing that should only ever be done in truly exceptional cases by a repository admin.

-1 to ever enabling revprop changes.
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Request: Changing commit message in llvm trunk

Sanjoy Das
In reply to this post by Tim Northover-2
On Tue, Jul 28, 2015 at 10:01 PM, Tim Northover <[hidden email]> wrote:
>>> This will surely break git mirrors.
>>
>> Sounds like the git mirror's problem.
>
> And every user of those mirrors, unless we have the resources to forge
> a SHA1 hash for the new commit message.

Caveat: I'm not a git expert, but we could export the changed svn
commit message as a "note" to the corresponding git commit.

>
> Tim.
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Request: Changing commit message in llvm trunk

Sean Silva-2
In reply to this post by Martell Malone


On Tue, Jul 28, 2015 at 10:42 AM, Martell Malone <[hidden email]> wrote:
Hi

I'm trying to change a commit message that became malformed when pushing to trunk

svn propedit svn:log --revprop -r 243434
svn: E175002: DAV request failed; it's possible that the repository's pre-revprop-change hook either failed or is non-existent
svn: E175002: Repository has not been enabled to accept revision propchanges;
ask the administrator to create a pre-revprop-change hook

As described here commit messages are not versioned on svn
Would it be possible to get a hook made for this
http://stackoverflow.com/questions/692851/can-i-go-back-and-edit-comments-on-an-svn-checkin

It could be very useful for 3 reason.

1.
Currently if someone makes a commit and does not have it is not automatically closed by phabracator by adding
Differential Revision: http://reviews.llvm.org/DXXXXX
at the end of their commit
The committer then often has to go back and manually enter a message to tell someone that their patch has landed

With a hook for this it would be easy to update the message invoking phabracator to now close the revision as it has landed.

we could even quite easily create a script for this for any committer to use to speed up the process, example here is a script called landed

landed D11511 r243434

Probably easier to have Phab just monitor the post-commit review thread on the mailing list.
 

2.
If a patch is reverted at some point because it is broken.
It would be nice to be able to edit the commit message in rXXX to tell the reader that the commit was reverted in rYYY and reapplied with a fix in rZZZ

Usually we chime in on the post-commit thread saying "btw, reverted in rXXXXXX".
 

3.
If you have a pesky typo in your commit like I have :)

I don't think anybody really cares very much about typos in the commit history, as long as they are clarified in the post-commit thread (just like if the code has a problem or whatever).

-- Sean Silva
 

I'm sure the hook could be made so that only an author can edit their own commit message or an admin that can.

Kind Regards
Martell



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Request: Changing commit message in llvm trunk

Martell Malone
Thanks for all the feedback guys

So rather than speculating about what could and might not break.

I am going to test all these different scenarios by creating my own svn server, svn mirror and git mirror etc
I'll get back to you all with details on what breaks and what doesn't soon with changing the commit msgs

Many Thanks
Martell

On Wed, Jul 29, 2015 at 7:01 AM, Sean Silva <[hidden email]> wrote:


On Tue, Jul 28, 2015 at 10:42 AM, Martell Malone <[hidden email]> wrote:
Hi

I'm trying to change a commit message that became malformed when pushing to trunk

svn propedit svn:log --revprop -r 243434
svn: E175002: DAV request failed; it's possible that the repository's pre-revprop-change hook either failed or is non-existent
svn: E175002: Repository has not been enabled to accept revision propchanges;
ask the administrator to create a pre-revprop-change hook

As described here commit messages are not versioned on svn
Would it be possible to get a hook made for this
http://stackoverflow.com/questions/692851/can-i-go-back-and-edit-comments-on-an-svn-checkin

It could be very useful for 3 reason.

1.
Currently if someone makes a commit and does not have it is not automatically closed by phabracator by adding
Differential Revision: http://reviews.llvm.org/DXXXXX
at the end of their commit
The committer then often has to go back and manually enter a message to tell someone that their patch has landed

With a hook for this it would be easy to update the message invoking phabracator to now close the revision as it has landed.

we could even quite easily create a script for this for any committer to use to speed up the process, example here is a script called landed

landed D11511 r243434

Probably easier to have Phab just monitor the post-commit review thread on the mailing list.
 

2.
If a patch is reverted at some point because it is broken.
It would be nice to be able to edit the commit message in rXXX to tell the reader that the commit was reverted in rYYY and reapplied with a fix in rZZZ

Usually we chime in on the post-commit thread saying "btw, reverted in rXXXXXX".
 

3.
If you have a pesky typo in your commit like I have :)

I don't think anybody really cares very much about typos in the commit history, as long as they are clarified in the post-commit thread (just like if the code has a problem or whatever).

-- Sean Silva
 

I'm sure the hook could be made so that only an author can edit their own commit message or an admin that can.

Kind Regards
Martell



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev




_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Request: Changing commit message in llvm trunk

Reid Kleckner-2
On Wed, Jul 29, 2015 at 9:04 AM, Martell Malone <[hidden email]> wrote:
Thanks for all the feedback guys

So rather than speculating about what could and might not break.

I am going to test all these different scenarios by creating my own svn server, svn mirror and git mirror etc
I'll get back to you all with details on what breaks and what doesn't soon with changing the commit msgs

Many Thanks
Martell

Even if you do discover some way to make git-svn mirrors support updates to past commit messages, I'm not sure we'd want to use it. I think most people would rather keep the history simple.

People make bad commits to LLVM all the time, and there's nothing wrong with that. The normal way we deal with it is to revert the change and commit it again with a fix. Bad commit messages aren't really that different from bad code, and they can be fixed the same way. :)

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Request: Changing commit message in llvm trunk

Andy Somogyi
Just a thought, and I'm sure this has been discussed before, but what about hosting on github?

GitHub provides an excellent svn bridge for people who still want to use svn.

We host a number of projects on github where we have a few devs who are more comfortable using an svn client and it seems to work very well.

On Jul 29, 2015, at 12:38 PM, Reid Kleckner <[hidden email]> wrote:

On Wed, Jul 29, 2015 at 9:04 AM, Martell Malone <[hidden email]> wrote:
Thanks for all the feedback guys

So rather than speculating about what could and might not break.

I am going to test all these different scenarios by creating my own svn server, svn mirror and git mirror etc
I'll get back to you all with details on what breaks and what doesn't soon with changing the commit msgs

Many Thanks
Martell

Even if you do discover some way to make git-svn mirrors support updates to past commit messages, I'm not sure we'd want to use it. I think most people would rather keep the history simple.

People make bad commits to LLVM all the time, and there's nothing wrong with that. The normal way we deal with it is to revert the change and commit it again with a fix. Bad commit messages aren't really that different from bad code, and they can be fixed the same way. :)
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev