Submitter | Stanislau Hlebik |
---|---|
Date | Oct. 4, 2016, 11:08 a.m. |
Message ID | <9069c255f60c9d34e5f4.1475579322@dev1918.lla1.facebook.com> |
Download | mbox | patch |
Permalink | /patch/16838/ |
State | Accepted |
Headers | show |
Comments
On Tue, 4 Oct 2016 04:08:42 -0700, Stanislau Hlebik wrote: > # HG changeset patch > # User Stanislau Hlebik <stash@fb.com> > # Date 1475579208 25200 > # Tue Oct 04 04:06:48 2016 -0700 > # Node ID 9069c255f60c9d34e5f45d690cfa9316fdc71722 > # Parent 1779dde4c9ef97cb242f8d501655f236f66e5439 > update: warn if cwd was deleted Looks good to me per comments on V3 and V4. Queued, thanks.
Stanislau Hlebik wrote: > diff --git a/tests/test-rebase-scenario-global.t b/tests/test-rebase-scenario-global.t > --- a/tests/test-rebase-scenario-global.t > +++ b/tests/test-rebase-scenario-global.t > @@ -758,6 +758,8 @@ > $ hg commit -m 'second source with subdir' > $ hg rebase -b . -d 1 --traceback > rebasing 2:779a07b1b7a0 "first source commit" > + current directory was removed > + (consider changing to repo root: $TESTTMP/cwd-vanish) > rebasing 3:a7d6f3a00bf3 "second source with subdir" (tip) > saved backup bundle to $TESTTMP/cwd-vanish/.hg/strip-backup/779a07b1b7a0-853e0073-backup.hg (glob) > > diff --git a/tests/test-update-names.t b/tests/test-update-names.t > --- a/tests/test-update-names.t > +++ b/tests/test-update-names.t > @@ -72,3 +72,15 @@ > $ cd .. > > #endif > + > +Test that warning is printed if cwd is deleted during update > + $ hg init r4 && cd r4 > + $ mkdir dir > + $ cd dir > + $ echo a > a > + $ echo b > b > + $ hg add a b > + $ hg ci -m "file and dir" > + $ hg up -q null > + current directory was removed > + (consider changing to repo root: $TESTTMP/r1/r4) So these tests are causing problems on Solaris. Our rmdir() returns EINVAL when trying to remove the cwd, even if you're not specifically trying to remove "." (at least on ZFS). That error is caught silently handled elsewhere, though it does leave the directory behind once the command is complete. But it does mean that when we get to the getcwd(), the directory is still there, so the error doesn't happen. I'm not entirely sure what to do here. I'm also not really sure what the driver for this fix was, so I'm not sure if suggesting something like dropping the warning in favor of silently chdir()ing to the repo root would be sufficient. Thoughts? Thanks, Danek
The fix is not that important so I think it’s fine to revert it if it causes troubles. On 10/24/16, 11:20 PM, "Danek Duvall" <danek.duvall@oracle.com> wrote: Stanislau Hlebik wrote: > diff --git a/tests/test-rebase-scenario-global.t b/tests/test-rebase-scenario-global.t > --- a/tests/test-rebase-scenario-global.t > +++ b/tests/test-rebase-scenario-global.t > @@ -758,6 +758,8 @@ > $ hg commit -m 'second source with subdir' > $ hg rebase -b . -d 1 --traceback > rebasing 2:779a07b1b7a0 "first source commit" > + current directory was removed > + (consider changing to repo root: $TESTTMP/cwd-vanish) > rebasing 3:a7d6f3a00bf3 "second source with subdir" (tip) > saved backup bundle to $TESTTMP/cwd-vanish/.hg/strip-backup/779a07b1b7a0-853e0073-backup.hg (glob) > > diff --git a/tests/test-update-names.t b/tests/test-update-names.t > --- a/tests/test-update-names.t > +++ b/tests/test-update-names.t > @@ -72,3 +72,15 @@ > $ cd .. > > #endif > + > +Test that warning is printed if cwd is deleted during update > + $ hg init r4 && cd r4 > + $ mkdir dir > + $ cd dir > + $ echo a > a > + $ echo b > b > + $ hg add a b > + $ hg ci -m "file and dir" > + $ hg up -q null > + current directory was removed > + (consider changing to repo root: $TESTTMP/r1/r4) So these tests are causing problems on Solaris. Our rmdir() returns EINVAL when trying to remove the cwd, even if you're not specifically trying to remove "." (at least on ZFS). That error is caught silently handled elsewhere, though it does leave the directory behind once the command is complete. But it does mean that when we get to the getcwd(), the directory is still there, so the error doesn't happen. I'm not entirely sure what to do here. I'm also not really sure what the driver for this fix was, so I'm not sure if suggesting something like dropping the warning in favor of silently chdir()ing to the repo root would be sufficient. Thoughts? Thanks, Danek
Patch
diff --git a/mercurial/merge.py b/mercurial/merge.py --- a/mercurial/merge.py +++ b/mercurial/merge.py @@ -1036,6 +1036,12 @@ unlink = util.unlinkpath wjoin = repo.wjoin audit = repo.wvfs.audit + try: + cwd = os.getcwd() + except OSError as err: + if err.errno != errno.ENOENT: + raise + cwd = None i = 0 for f, args, msg in actions: repo.ui.debug(" %s: %s -> r\n" % (f, msg)) @@ -1053,6 +1059,18 @@ i += 1 if i > 0: yield i, f + if cwd: + # cwd was present before we started to remove files + # let's check if it is present after we removed them + try: + os.getcwd() + except OSError as err: + if err.errno != errno.ENOENT: + raise + # Print a warning if cwd was deleted + repo.ui.warn(_("current directory was removed\n" + "(consider changing to repo root: %s)\n") % + repo.root) def batchget(repo, mctx, actions): """apply gets to the working directory diff --git a/tests/test-rebase-scenario-global.t b/tests/test-rebase-scenario-global.t --- a/tests/test-rebase-scenario-global.t +++ b/tests/test-rebase-scenario-global.t @@ -758,6 +758,8 @@ $ hg commit -m 'second source with subdir' $ hg rebase -b . -d 1 --traceback rebasing 2:779a07b1b7a0 "first source commit" + current directory was removed + (consider changing to repo root: $TESTTMP/cwd-vanish) rebasing 3:a7d6f3a00bf3 "second source with subdir" (tip) saved backup bundle to $TESTTMP/cwd-vanish/.hg/strip-backup/779a07b1b7a0-853e0073-backup.hg (glob) diff --git a/tests/test-update-names.t b/tests/test-update-names.t --- a/tests/test-update-names.t +++ b/tests/test-update-names.t @@ -72,3 +72,15 @@ $ cd .. #endif + +Test that warning is printed if cwd is deleted during update + $ hg init r4 && cd r4 + $ mkdir dir + $ cd dir + $ echo a > a + $ echo b > b + $ hg add a b + $ hg ci -m "file and dir" + $ hg up -q null + current directory was removed + (consider changing to repo root: $TESTTMP/r1/r4)