| Submitter | Laurent Charignon |
|---|---|
| Date | April 22, 2015, 9:57 p.m. |
| Message ID | <827224ee690234eb78e3.1429739826@lcharignon-mbp.local> |
| Download | mbox | patch |
| Permalink | /patch/8766/ |
| State | Accepted |
| Commit | edf907bd8144544cb2c00fc074c4591a112d698c |
| Headers | show |
Comments
On Wed, 2015-04-22 at 14:57 -0700, Laurent Charignon wrote: > # HG changeset patch > # User Laurent Charignon <lcharignon@fb.com> > # Date 1429736190 25200 > # Wed Apr 22 13:56:30 2015 -0700 > # Branch stable > # Node ID 827224ee690234eb78e3c12ddcc381879bfba014 > # Parent 1f9127c9239b9c39c676bb09752db1e2ca6c26f7 > record: fix record with change on moved file crashes (issue4619) > reverting 79fceed67676, add a test to prevent the issue from coming back. So.. this means we lose the fix to the legitimate bug that was in that change ("record: allow editing new files (issue4304)"), right? What's the plan here?
On 4/22/15, 5:22 PM, "Matt Mackall" <mpm@selenic.com> wrote: >On Wed, 2015-04-22 at 14:57 -0700, Laurent Charignon wrote: >> # HG changeset patch >> # User Laurent Charignon <lcharignon@fb.com> >> # Date 1429736190 25200 >> # Wed Apr 22 13:56:30 2015 -0700 >> # Branch stable >> # Node ID 827224ee690234eb78e3c12ddcc381879bfba014 >> # Parent 1f9127c9239b9c39c676bb09752db1e2ca6c26f7 >> record: fix record with change on moved file crashes (issue4619) > >> reverting 79fceed67676, add a test to prevent the issue from coming >>back. > >So.. this means we lose the fix to the legitimate bug that was in that >change ("record: allow editing new files (issue4304)"), right? Correct > What's >the plan here? Since this issue is a regression and I didn't feel confident getting the right fix by the end of the week, I preferred rolling back the change that caused it. I will reopen issue4304 and work on the correct fix after, if that works for you. Thanks, Laurent > >-- >Mathematics is the supreme nostalgia of our time. > >
On Thu, 2015-04-23 at 17:11 +0000, Laurent Charignon wrote: > > On 4/22/15, 5:22 PM, "Matt Mackall" <mpm@selenic.com> wrote: > > >On Wed, 2015-04-22 at 14:57 -0700, Laurent Charignon wrote: > >> # HG changeset patch > >> # User Laurent Charignon <lcharignon@fb.com> > >> # Date 1429736190 25200 > >> # Wed Apr 22 13:56:30 2015 -0700 > >> # Branch stable > >> # Node ID 827224ee690234eb78e3c12ddcc381879bfba014 > >> # Parent 1f9127c9239b9c39c676bb09752db1e2ca6c26f7 > >> record: fix record with change on moved file crashes (issue4619) > > > >> reverting 79fceed67676, add a test to prevent the issue from coming > >>back. > > > >So.. this means we lose the fix to the legitimate bug that was in that > >change ("record: allow editing new files (issue4304)"), right? > Correct > > What's > >the plan here? > > Since this issue is a regression and I didn't feel confident getting the > right fix by the end of the week, I preferred rolling back the change that > caused it. > I will reopen issue4304 and work on the correct fix after, if that works > for you. Ok, sounds good. I just like to know what the plan is when we're trading one fix for another. This is queued for stable.
On 04/23/2015 06:34 PM, Matt Mackall wrote: > On Thu, 2015-04-23 at 17:11 +0000, Laurent Charignon wrote: >> >> On 4/22/15, 5:22 PM, "Matt Mackall" <mpm@selenic.com> wrote: >> >>> On Wed, 2015-04-22 at 14:57 -0700, Laurent Charignon wrote: >>>> # HG changeset patch >>>> # User Laurent Charignon <lcharignon@fb.com> >>>> # Date 1429736190 25200 >>>> # Wed Apr 22 13:56:30 2015 -0700 >>>> # Branch stable >>>> # Node ID 827224ee690234eb78e3c12ddcc381879bfba014 >>>> # Parent 1f9127c9239b9c39c676bb09752db1e2ca6c26f7 >>>> record: fix record with change on moved file crashes (issue4619) >>> >>>> reverting 79fceed67676, add a test to prevent the issue from coming >>>> back. >>> >>> So.. this means we lose the fix to the legitimate bug that was in that >>> change ("record: allow editing new files (issue4304)"), right? >> Correct >>> What's >>> the plan here? >> >> Since this issue is a regression and I didn't feel confident getting the >> right fix by the end of the week, I preferred rolling back the change that >> caused it. >> I will reopen issue4304 and work on the correct fix after, if that works >> for you. > > Ok, sounds good. I just like to know what the plan is when we're trading > one fix for another. This is queued for stable. If we "unfix" the new file issue, we should probably mark `commit -i` as "experimental" (DEPRECATED) because it is not ready for prime time without it. >
On Fri, Apr 24, 2015 at 12:07:18AM +0100, Pierre-Yves David wrote: > > > On 04/23/2015 06:34 PM, Matt Mackall wrote: > >On Thu, 2015-04-23 at 17:11 +0000, Laurent Charignon wrote: > >> > >>On 4/22/15, 5:22 PM, "Matt Mackall" <mpm@selenic.com> wrote: > >> > >>>On Wed, 2015-04-22 at 14:57 -0700, Laurent Charignon wrote: > >>>># HG changeset patch > >>>># User Laurent Charignon <lcharignon@fb.com> > >>>># Date 1429736190 25200 > >>>># Wed Apr 22 13:56:30 2015 -0700 > >>>># Branch stable > >>>># Node ID 827224ee690234eb78e3c12ddcc381879bfba014 > >>>># Parent 1f9127c9239b9c39c676bb09752db1e2ca6c26f7 > >>>>record: fix record with change on moved file crashes (issue4619) > >>> > >>>>reverting 79fceed67676, add a test to prevent the issue from coming > >>>>back. > >>> > >>>So.. this means we lose the fix to the legitimate bug that was in that > >>>change ("record: allow editing new files (issue4304)"), right? > >>Correct > >>>What's > >>>the plan here? > >> > >>Since this issue is a regression and I didn't feel confident getting the > >>right fix by the end of the week, I preferred rolling back the change that > >>caused it. > >>I will reopen issue4304 and work on the correct fix after, if that works > >>for you. > > > >Ok, sounds good. I just like to know what the plan is when we're trading > >one fix for another. This is queued for stable. > > If we "unfix" the new file issue, we should probably mark `commit -i` as > "experimental" (DEPRECATED) because it is not ready for prime time without > it. This makes sense to me. > > > > > -- > Pierre-Yves David > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
On 04/27/2015 01:56 PM, Augie Fackler wrote: > On Fri, Apr 24, 2015 at 12:07:18AM +0100, Pierre-Yves David wrote: >> >> >> On 04/23/2015 06:34 PM, Matt Mackall wrote: >>> On Thu, 2015-04-23 at 17:11 +0000, Laurent Charignon wrote: >>>> >>>> On 4/22/15, 5:22 PM, "Matt Mackall" <mpm@selenic.com> wrote: >>>> >>>>> On Wed, 2015-04-22 at 14:57 -0700, Laurent Charignon wrote: >>>>>> # HG changeset patch >>>>>> # User Laurent Charignon <lcharignon@fb.com> >>>>>> # Date 1429736190 25200 >>>>>> # Wed Apr 22 13:56:30 2015 -0700 >>>>>> # Branch stable >>>>>> # Node ID 827224ee690234eb78e3c12ddcc381879bfba014 >>>>>> # Parent 1f9127c9239b9c39c676bb09752db1e2ca6c26f7 >>>>>> record: fix record with change on moved file crashes (issue4619) >>>>> >>>>>> reverting 79fceed67676, add a test to prevent the issue from coming >>>>>> back. >>>>> >>>>> So.. this means we lose the fix to the legitimate bug that was in that >>>>> change ("record: allow editing new files (issue4304)"), right? >>>> Correct >>>>> What's >>>>> the plan here? >>>> >>>> Since this issue is a regression and I didn't feel confident getting the >>>> right fix by the end of the week, I preferred rolling back the change that >>>> caused it. >>>> I will reopen issue4304 and work on the correct fix after, if that works >>>> for you. >>> >>> Ok, sounds good. I just like to know what the plan is when we're trading >>> one fix for another. This is queued for stable. >> >> If we "unfix" the new file issue, we should probably mark `commit -i` as >> "experimental" (DEPRECATED) because it is not ready for prime time without >> it. > > This makes sense to me The unfixed bug have apparently been refixed. .-- Pierre-Yves David
Patch
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py --- a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -59,8 +59,6 @@ def dorecord(ui, repo, commitfunc, cmdsuggest, backupall, filterfn, *pats, **opts): import merge as mergemod - hunkclasses = (crecordmod.uihunk, patch.recordhunk) - ishunk = lambda x: isinstance(x, hunkclasses) if not ui.interactive(): raise util.Abort(_('running non-interactively, use %s instead') % @@ -117,12 +115,6 @@ ui.status(_('no changes to record\n')) return 0 - newandmodifiedfiles = set() - for h in chunks: - isnew = h.filename() in status.added - if ishunk(h) and isnew and not h in originalchunks: - newandmodifiedfiles.add(h.filename()) - modified = set(status.modified) # 2. backup changed files, so we can restore them in the end @@ -130,8 +122,7 @@ if backupall: tobackup = changed else: - tobackup = [f for f in newfiles - if f in modified or f in newandmodifiedfiles] + tobackup = [f for f in newfiles if f in modified] backups = {} if tobackup: @@ -155,13 +146,11 @@ fp = cStringIO.StringIO() for c in chunks: fname = c.filename() - if fname in backups or fname in newandmodifiedfiles: + if fname in backups: c.write(fp) dopatch = fp.tell() fp.seek(0) - [os.unlink(c) for c in newandmodifiedfiles] - # 3a. apply filtered patch to clean repo (clean) if backups: # Equivalent to hg.revert diff --git a/mercurial/patch.py b/mercurial/patch.py --- a/mercurial/patch.py +++ b/mercurial/patch.py @@ -820,7 +820,7 @@ """ diffgit_re = re.compile('diff --git a/(.*) b/(.*)$') diff_re = re.compile('diff -r .* (.*)$') - allhunks_re = re.compile('(?:index|deleted file) ') + allhunks_re = re.compile('(?:index|new file|deleted file) ') pretty_re = re.compile('(?:new file|deleted file) ') special_re = re.compile('(?:index|new|deleted|copy|rename) ') diff --git a/tests/test-commit-interactive-curses.t b/tests/test-commit-interactive-curses.t --- a/tests/test-commit-interactive-curses.t +++ b/tests/test-commit-interactive-curses.t @@ -143,48 +143,10 @@ 10 y -Editing patch of newly added file - - $ cat > editor.sh << '__EOF__' - > cat "$1" | sed "s/first/very/g" > tt - > mv tt "$1" - > __EOF__ - $ cat > newfile << '__EOF__' - > This is the first line - > This is the second line - > This is the third line - > __EOF__ - $ hg add newfile - $ cat <<EOF >testModeCommands - > f - > KEY_DOWN - > KEY_DOWN - > KEY_DOWN - > e - > X - > EOF - $ HGEDITOR="\"sh\" \"`pwd`/editor.sh\"" hg commit -i -d '23 0' -medit-patch-new - $ hg tip - changeset: 4:6a0a43e9eff5 - tag: tip - user: test - date: Thu Jan 01 00:00:23 1970 +0000 - summary: edit-patch-new - - $ hg cat -r tip newfile - This is the very line - This is the second line - This is the third line - - $ cat newfile - This is the first line - This is the second line - This is the third line - Newly added files can be selected with the curses interface $ hg update -C . - 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + 0 files updated, 0 files merged, 0 files removed, 0 files unresolved $ echo "hello" > x $ hg add x $ cat <<EOF >testModeCommands @@ -194,10 +156,8 @@ > EOF $ hg st A x - ? editor.sh ? testModeCommands $ hg commit -i -m "newly added file" -d "0 0" $ hg st - ? editor.sh ? testModeCommands diff --git a/tests/test-commit-interactive.t b/tests/test-commit-interactive.t --- a/tests/test-commit-interactive.t +++ b/tests/test-commit-interactive.t @@ -229,25 +229,11 @@ $ hg add plain $ hg commit -i -d '7 0' -m plain plain<<EOF > y - > y > EOF diff --git a/plain b/plain new file mode 100644 examine changes to 'plain'? [Ynesfdaq?] y - @@ -0,0 +1,10 @@ - +1 - +2 - +3 - +4 - +5 - +6 - +7 - +8 - +9 - +10 - record this change to 'plain'? [Ynesfdaq?] y - $ hg tip -p changeset: 7:11fb457c1be4 tag: tip @@ -348,10 +334,6 @@ new file mode 100644 examine changes to 'plain2'? [Ynesfdaq?] y - @@ -0,0 +1,1 @@ - +1 - record change 2/2 to 'plain2'? [Ynesfdaq?] y - Modify beginning, trim end, record both, add another file to test changes numbering @@ -1395,40 +1377,34 @@ $ export HGUSER -Editing patch of newly added file +Moving files - $ cat > editor.sh << '__EOF__' - > cat "$1" | sed "s/first/very/g" > tt - > mv tt "$1" - > __EOF__ - $ cat > newfile << '__EOF__' - > This is the first line - > This is the second line - > This is the third line - > __EOF__ - $ hg add newfile - $ HGEDITOR="\"sh\" \"`pwd`/editor.sh\"" hg commit -i -d '23 0' -medit-patch-new <<EOF + $ hg update -C . + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + $ hg mv plain plain3 + $ echo somechange >> plain3 + $ hg commit -i -d '23 0' -mmoving_files << EOF > y - > e + > y > EOF - diff --git a/newfile b/newfile - new file mode 100644 - examine changes to 'newfile'? [Ynesfdaq?] y + diff --git a/plain b/plain3 + rename from plain + rename to plain3 + 1 hunks, 1 lines changed + examine changes to 'plain' and 'plain3'? [Ynesfdaq?] y - @@ -0,0 +1,3 @@ - +This is the first line - +This is the second line - +This is the third line - record this change to 'newfile'? [Ynesfdaq?] e + @@ -11,3 +11,4 @@ + 9 + 10 + 11 + +somechange + record this change to 'plain3'? [Ynesfdaq?] y - $ hg cat -r tip newfile - This is the very line - This is the second line - This is the third line - - $ cat newfile - This is the first line - This is the second line - This is the third line - + $ hg tip + changeset: 30:542e1f362a22 + tag: tip + user: test + date: Thu Jan 01 00:00:23 1970 +0000 + summary: moving_files + $ cd .. diff --git a/tests/test-keyword.t b/tests/test-keyword.t --- a/tests/test-keyword.t +++ b/tests/test-keyword.t @@ -479,12 +479,6 @@ new file mode 100644 examine changes to 'r'? [Ynesfdaq?] y - @@ -0,0 +1,1 @@ - +$Id$ - record this change to 'r'? [Ynesfdaq?] y - - resolving manifests - patching file r committing files: r committing manifest @@ -513,12 +507,6 @@ new file mode 100644 examine changes to 'i'? [Ynesfdaq?] y - @@ -0,0 +1,1 @@ - +$Id$ - record this change to 'i'? [Ynesfdaq?] y - - resolving manifests - patching file i committing files: i committing manifest diff --git a/tests/test-mq-subrepo.t b/tests/test-mq-subrepo.t --- a/tests/test-mq-subrepo.t +++ b/tests/test-mq-subrepo.t @@ -295,11 +295,6 @@ new file mode 100644 examine changes to '.hgsub'? [Ynesfdaq?] y - @@ -0,0 +1,1 @@ - +sub = sub - record this change to '.hgsub'? [Ynesfdaq?] y - - warning: subrepo spec file '.hgsub' not found abort: uncommitted changes in subrepository 'sub' [255] % update substate when adding .hgsub w/clean updated subrepo @@ -309,11 +304,6 @@ new file mode 100644 examine changes to '.hgsub'? [Ynesfdaq?] y - @@ -0,0 +1,1 @@ - +sub = sub - record this change to '.hgsub'? [Ynesfdaq?] y - - warning: subrepo spec file '.hgsub' not found path sub source sub revision b2fdb12cd82b021c3b7053d67802e77b6eeaee31