| Submitter | Boris Penev |
|---|---|
| Date | July 30, 2014, 1:53 a.m. |
| Message ID | <4076ff9a7bb016e779da.1406685195@localhost.localdomain> |
| Download | mbox | patch |
| Permalink | /patch/5209/ |
| State | Rejected |
| Headers | show |
Comments
On Tue, Jul 29, 2014 at 9:53 PM, Boris Penev <bob53181@yahoo.com> wrote: > # HG changeset patch > # User Boris Penev <bob53181@yahoo.com> > # Date 1406685063 -3600 > # Wed Jul 30 02:51:03 2014 +0100 > # Branch stable > # Node ID 4076ff9a7bb016e779da879c77b2de2e0e79a082 > # Parent ad56fc55cbc3870d257e163469c687088627283b > histedit: remove history_source No thanks. > History source is polluting the commit messages. This is > visible when converting the repository to git but the fact that > it stays in the history is bad enough by itself. Ignoring hg-git, why do you care about some invisible metadata in your history? > This patch > removes the needless logging. I do not know why you would need > this but it possibly could be non-default, activated by an > argument to the histedit command. > > diff -r ad56fc55cbc3 -r 4076ff9a7bb0 hgext/histedit.py > --- a/hgext/histedit.py Mon Jul 28 10:05:17 2014 +0200 > +++ b/hgext/histedit.py Wed Jul 30 02:51:03 2014 +0100 > @@ -188,8 +188,6 @@ > > This function ensure we apply the same treatment to all changesets. > > - - Add a 'histedit_source' entry in extra. > - > Note that fold have its own separated logic because its handling is a bit > different and not easily factored out of the fold method. > """ > @@ -199,9 +197,6 @@ > try: > repo.ui.setconfig('phases', 'new-commit', phasemin, > 'histedit') > - extra = kwargs.get('extra', {}).copy() > - extra['histedit_source'] = src.hex() > - kwargs['extra'] = extra > return repo.commit(**kwargs) > finally: > repo.ui.restoreconfig(phasebackup) > @@ -289,7 +284,6 @@ > message = first.description() > user = commitopts.get('user') > date = commitopts.get('date') > - extra = commitopts.get('extra') > > parents = (first.p1().node(), first.p2().node()) > new = context.memctx(repo, > @@ -299,7 +293,6 @@ > filectxfn=filectxfn, > user=user, > date=date, > - extra=extra, > editor=cmdutil.getcommiteditor(edit=True)) > return repo.commitctx(new) > > @@ -316,7 +309,7 @@ > # drop the second merge parent > commit = commitfuncfor(repo, oldctx) > n = commit(text=oldctx.description(), user=oldctx.user(), > - date=oldctx.date(), extra=oldctx.extra()) > + date=oldctx.date()) > if n is None: > ui.warn(_('%s: empty changeset\n') > % node.hex(ha)) > @@ -341,7 +334,7 @@ > raise error.InterventionRequired( > _('Fix up the change and run hg histedit --continue')) > n = repo.commit(text='fold-temp-revision %s' % ha, user=oldctx.user(), > - date=oldctx.date(), extra=oldctx.extra()) > + date=oldctx.date()) > if n is None: > ui.warn(_('%s: empty changeset') > % node.hex(ha)) > @@ -367,12 +360,6 @@ > commitopts['message'] = newmessage > # date > commitopts['date'] = max(ctx.date(), oldctx.date()) > - extra = ctx.extra().copy() > - # histedit_source > - # note: ctx is likely a temporary commit but that the best we can do here > - # This is sufficient to solve issue3681 anyway > - extra['histedit_source'] = '%s,%s' % (ctx.hex(), oldctx.hex()) > - commitopts['extra'] = extra > phasebackup = repo.ui.backupconfig('phases', 'new-commit') > try: > phasemin = max(ctx.phase(), oldctx.phase()) > @@ -405,7 +392,6 @@ > message = oldctx.description() > commit = commitfuncfor(repo, oldctx) > new = commit(text=message, user=oldctx.user(), date=oldctx.date(), > - extra=oldctx.extra(), > editor=cmdutil.getcommiteditor(edit=True)) > newctx = repo[new] > if oldctx.node() != newctx.node(): > @@ -686,7 +672,7 @@ > editor = cmdutil.getcommiteditor(edit=editopt) > commit = commitfuncfor(repo, ctx) > new = commit(text=message, user=ctx.user(), > - date=ctx.date(), extra=ctx.extra(), > + date=ctx.date(), > editor=editor) > if new is not None: > newchildren.append(new) > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
On 07/29/2014 06:53 PM, Boris Penev wrote: > # HG changeset patch > # User Boris Penev <bob53181@yahoo.com> > # Date 1406685063 -3600 > # Wed Jul 30 02:51:03 2014 +0100 > # Branch stable > # Node ID 4076ff9a7bb016e779da879c77b2de2e0e79a082 > # Parent ad56fc55cbc3870d257e163469c687088627283b > histedit: remove history_source > > History source is polluting the commit messages. This is > visible when converting the repository to git but the fact that > it stays in the history is bad enough by itself. This patch > removes the needless logging. I do not know why you would need > this but it possibly could be non-default, activated by an > argument to the histedit command. This change is actually useful for at least two things: 1) keeping relevant meta data when using --keep 2) Ensuring the a pure happen only history creation. A property hihgly critical to any distributed system Doesn't git have a way to quietly store arbitraty metadata?
On 07/30/2014 11:21 AM, Pierre-Yves David wrote: > 2) Ensuring the a pure happen only history creation. A property hihgly > critical to any distributed system "pure happen only" should have been "pure append only"
On 07/30/2014 11:21 AM, Pierre-Yves David wrote: > > This change is actually useful for at least two things: > > 1) keeping relevant meta data when using --keep > 2) Ensuring the a pure happen only history creation. A property hihgly > critical to any distributed system > > Doesn't git have a way to quietly store arbitraty metadata? There are Git notes, but they're not an input to the commit hash so storing hg metadata in there would mean hg-git loses its 1:1 mapping between Mercurial and Git revisions. Other than that I'm not aware of anything.
On Jul 30, 2014, at 2:54 PM, Siddharth Agarwal <sid@less-broken.com> wrote: > On 07/30/2014 11:21 AM, Pierre-Yves David wrote: >> >> This change is actually useful for at least two things: >> >> 1) keeping relevant meta data when using --keep >> 2) Ensuring the a pure happen only history creation. A property hihgly critical to any distributed system >> >> Doesn't git have a way to quietly store arbitraty metadata? > > There are Git notes, but they're not an input to the commit hash so storing hg metadata in there would mean hg-git loses its 1:1 mapping between Mercurial and Git revisions. Other than that I'm not aware of anything. I did just notice that a noop histedit seemed to rewrite shas - that's definitely a bug that we should fix. (I was in the middle of a big project, so I didn't pursue it when I saw it.)
On Thu Jul 31 07:03:03 CDT 2014, Augie Fackler wrote: > I did just notice that a noop histedit seemed to rewrite shas - > that's definitely a bug that we should fix. > > (I was in the middle of a big project, so I didn't pursue it when I > saw it.) Does it mean that the proposed behavior will be added to histedit (either as primary/default or secondary)? I expected a more concrete yes or no for the time passed.
On 08/20/2014 03:28 PM, Boris Penev wrote: > On Thu Jul 31 07:03:03 CDT 2014, Augie Fackler wrote: >> I did just notice that a noop histedit seemed to rewrite shas - >> that's definitely a bug that we should fix. >> >> (I was in the middle of a big project, so I didn't pursue it when I >> saw it.) > > Does it mean that the proposed behavior will be added to histedit > (either as primary/default or secondary)? I expected a more concrete yes > or no for the time passed. Can you be more precise about what you mean by "the proposed behavior" ?
On 08/21/2014 01:45 AM, Pierre-Yves David wrote: > On 08/20/2014 03:28 PM, Boris Penev wrote: >> Does it mean that the proposed behavior will be added to histedit >> (either as primary/default or secondary)? I expected a more >> concrete yes or no for the time passed. > > Can you be more precise about what you mean by "the proposed > behavior" ? I mean to not keep irrelevant metadata in the commit messages. Maybe triggered by --keep=false or something like that. Often you really do not need to keep such information in your repository and it is particularly annoying when keeping a git mirror (all affected commits show histedit_source information in git log).
On Thu, 2014-08-21 at 14:09 +0300, Boris Penev wrote: > On 08/21/2014 01:45 AM, Pierre-Yves David wrote: > > On 08/20/2014 03:28 PM, Boris Penev wrote: > >> Does it mean that the proposed behavior will be added to histedit > >> (either as primary/default or secondary)? I expected a more > >> concrete yes or no for the time passed. > > > > Can you be more precise about what you mean by "the proposed > > behavior" ? > > I mean to not keep irrelevant metadata in the commit messages. It's not irrelevant, and they're not in the commit messages (at least, not in hg). They are useful for when you want to restore history. Augie didn't decide to add these fields to the histedited commit just to annoy you. They're sort like a precambrian obsolescence marker like what you see with hg evolve. I think the real problem is that when you push with hg-git to a git repo, hg-git will translate extra: fields into lines of the git commit message. I don't like this either, and *in this context* it is irrelevant (git users typically don't care about hg's hashes). In that case, I think your code-killing wrath should be better directed to these lines,: https://bitbucket.org/durin42/hg-git/src/83e1c73136251271d8acbf92e05100ea175f904b/hggit/git_handler.py?at=default#cl-599 Also, those lines have a purpose too: to preserve all of the data in the round-trip of the git<->hg conversion. I think the best thing to do is to add an option to hg-git to not send that data to git for when you don't care about that round trip, or to make hg-git add that data in a location where the git UI won't show it to most git users. Kiln Harmony does this by storing it in *git*'s extra field: http://blog.fogcreek.com/kiln-harmony-internals-the-basics/ - Jordi G. H.
Patch
diff -r ad56fc55cbc3 -r 4076ff9a7bb0 hgext/histedit.py --- a/hgext/histedit.py Mon Jul 28 10:05:17 2014 +0200 +++ b/hgext/histedit.py Wed Jul 30 02:51:03 2014 +0100 @@ -188,8 +188,6 @@ This function ensure we apply the same treatment to all changesets. - - Add a 'histedit_source' entry in extra. - Note that fold have its own separated logic because its handling is a bit different and not easily factored out of the fold method. """ @@ -199,9 +197,6 @@ try: repo.ui.setconfig('phases', 'new-commit', phasemin, 'histedit') - extra = kwargs.get('extra', {}).copy() - extra['histedit_source'] = src.hex() - kwargs['extra'] = extra return repo.commit(**kwargs) finally: repo.ui.restoreconfig(phasebackup) @@ -289,7 +284,6 @@ message = first.description() user = commitopts.get('user') date = commitopts.get('date') - extra = commitopts.get('extra') parents = (first.p1().node(), first.p2().node()) new = context.memctx(repo, @@ -299,7 +293,6 @@ filectxfn=filectxfn, user=user, date=date, - extra=extra, editor=cmdutil.getcommiteditor(edit=True)) return repo.commitctx(new) @@ -316,7 +309,7 @@ # drop the second merge parent commit = commitfuncfor(repo, oldctx) n = commit(text=oldctx.description(), user=oldctx.user(), - date=oldctx.date(), extra=oldctx.extra()) + date=oldctx.date()) if n is None: ui.warn(_('%s: empty changeset\n') % node.hex(ha)) @@ -341,7 +334,7 @@ raise error.InterventionRequired( _('Fix up the change and run hg histedit --continue')) n = repo.commit(text='fold-temp-revision %s' % ha, user=oldctx.user(), - date=oldctx.date(), extra=oldctx.extra()) + date=oldctx.date()) if n is None: ui.warn(_('%s: empty changeset') % node.hex(ha)) @@ -367,12 +360,6 @@ commitopts['message'] = newmessage # date commitopts['date'] = max(ctx.date(), oldctx.date()) - extra = ctx.extra().copy() - # histedit_source - # note: ctx is likely a temporary commit but that the best we can do here - # This is sufficient to solve issue3681 anyway - extra['histedit_source'] = '%s,%s' % (ctx.hex(), oldctx.hex()) - commitopts['extra'] = extra phasebackup = repo.ui.backupconfig('phases', 'new-commit') try: phasemin = max(ctx.phase(), oldctx.phase()) @@ -405,7 +392,6 @@ message = oldctx.description() commit = commitfuncfor(repo, oldctx) new = commit(text=message, user=oldctx.user(), date=oldctx.date(), - extra=oldctx.extra(), editor=cmdutil.getcommiteditor(edit=True)) newctx = repo[new] if oldctx.node() != newctx.node(): @@ -686,7 +672,7 @@ editor = cmdutil.getcommiteditor(edit=editopt) commit = commitfuncfor(repo, ctx) new = commit(text=message, user=ctx.user(), - date=ctx.date(), extra=ctx.extra(), + date=ctx.date(), editor=editor) if new is not None: newchildren.append(new)