This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4419

rename: return error status if any rename/copy failed
ClosedPublic

Authored by martinvonz on Aug 29 2018, 1:47 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG5b92a717bfc1: rename: return error status if any rename/copy failed
Summary

Ever since 447ea621e50e (copy: propagate errors properly, 2007-12-06),
we have returned an error status if the source file did not
exist. That commit did not return error status for any other errors,
and it's unclear if that was on purpose or not. It seems to me like we
should return an error in the other cases to, so that's what this
patch does.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

martinvonz created this revision.Aug 29 2018, 1:47 PM
pulkit accepted this revision.Aug 29 2018, 3:29 PM
pulkit added a subscriber: pulkit.
pulkit added inline comments.
tests/test-rename.t
343

Not sure how I feel about this one. There are few operations which succeed and few which does not.

Closed by commit rHG5b92a717bfc1: rename: return error status if any rename/copy failed (authored by martinvonz). · Explain WhyAug 29 2018, 3:41 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/cmdutil.py (8 lines)
Mtests/test-copy.t (4 lines)
Mtests/test-rename.t (6 lines)

Diff 10647

mercurial/cmdutil.py

scmutil.checkportable(ui, abstarget) scmutil.checkportable(ui, abstarget)
# check for collisions # check for collisions
prevsrc = targets.get(abstarget) prevsrc = targets.get(abstarget)
if prevsrc is not None: if prevsrc is not None:
ui.warn(_('%s: not overwriting - %s collides with %s\n') % ui.warn(_('%s: not overwriting - %s collides with %s\n') %
(reltarget, repo.pathto(abssrc, cwd), (reltarget, repo.pathto(abssrc, cwd),
repo.pathto(prevsrc, cwd))) repo.pathto(prevsrc, cwd)))
return return True # report a failure
# check for overwrites # check for overwrites
exists = os.path.lexists(target) exists = os.path.lexists(target)
samefile = False samefile = False
if exists and abssrc != abstarget: if exists and abssrc != abstarget:
if (repo.dirstate.normalize(abssrc) == if (repo.dirstate.normalize(abssrc) ==
repo.dirstate.normalize(abstarget)): repo.dirstate.normalize(abstarget)):
if not rename: if not rename:
ui.warn(_("%s: can't copy - same file\n") % reltarget) ui.warn(_("%s: can't copy - same file\n") % reltarget)
return return True # report a failure
exists = False exists = False
samefile = True samefile = True
if not after and exists or after and state in 'mn': if not after and exists or after and state in 'mn':
if not opts['force']: if not opts['force']:
if state in 'mn': if state in 'mn':
msg = _('%s: not overwriting - file already committed\n') msg = _('%s: not overwriting - file already committed\n')
if after: if after:
else: else:
msg = _('%s: not overwriting - file exists\n') msg = _('%s: not overwriting - file exists\n')
if rename: if rename:
hint = _("('hg rename --after' to record the rename)\n") hint = _("('hg rename --after' to record the rename)\n")
else: else:
hint = _("('hg copy --after' to record the copy)\n") hint = _("('hg copy --after' to record the copy)\n")
ui.warn(msg % reltarget) ui.warn(msg % reltarget)
ui.warn(hint) ui.warn(hint)
return return True # report a failure
if after: if after:
if not exists: if not exists:
if rename: if rename:
ui.warn(_('%s: not recording move - %s does not exist\n') % ui.warn(_('%s: not recording move - %s does not exist\n') %
(relsrc, reltarget)) (relsrc, reltarget))
else: else:
ui.warn(_('%s: not recording copy - %s does not exist\n') % ui.warn(_('%s: not recording copy - %s does not exist\n') %
(relsrc, reltarget)) (relsrc, reltarget))
return return True # report a failure
elif not dryrun: elif not dryrun:
try: try:
if exists: if exists:
os.unlink(target) os.unlink(target)
targetdir = os.path.dirname(target) or '.' targetdir = os.path.dirname(target) or '.'
if not os.path.isdir(targetdir): if not os.path.isdir(targetdir):
os.makedirs(targetdir) os.makedirs(targetdir)
if samefile: if samefile:

tests/test-copy.t

$ hg mv foo foo3 $ hg mv foo foo3
foo: deleted in working directory foo: deleted in working directory
foo3 does not exist! foo3 does not exist!
$ hg up -qC . $ hg up -qC .
copy --after to a nonexistent target filename copy --after to a nonexistent target filename
$ hg cp -A foo dummy $ hg cp -A foo dummy
foo: not recording copy - dummy does not exist foo: not recording copy - dummy does not exist
[1]
dry-run; should show that foo is clean dry-run; should show that foo is clean
$ hg copy --dry-run foo bar $ hg copy --dry-run foo bar
$ hg st -A $ hg st -A
C foo C foo
should show copy should show copy
$ hg copy foo bar $ hg copy foo bar
$ hg st -C $ hg st -C
foo was clean: foo was clean:
$ hg st -AC foo $ hg st -AC foo
C foo C foo
Trying to copy on top of an existing file fails, Trying to copy on top of an existing file fails,
$ hg copy -A bar foo $ hg copy -A bar foo
foo: not overwriting - file already committed foo: not overwriting - file already committed
('hg copy --after --force' to replace the file by recording a copy) ('hg copy --after --force' to replace the file by recording a copy)
[1]
same error without the --after, so the user doesn't have to go through same error without the --after, so the user doesn't have to go through
two hints: two hints:
$ hg copy bar foo $ hg copy bar foo
foo: not overwriting - file already committed foo: not overwriting - file already committed
('hg copy --force' to replace the file by recording a copy) ('hg copy --force' to replace the file by recording a copy)
[1]
but it's considered modified after a copy --after --force but it's considered modified after a copy --after --force
$ hg copy -Af bar foo $ hg copy -Af bar foo
$ hg st -AC foo $ hg st -AC foo
M foo M foo
bar bar
The hint for a file that exists but is not in file history doesn't The hint for a file that exists but is not in file history doesn't
mention --force: mention --force:
$ touch xyzzy $ touch xyzzy
$ hg cp bar xyzzy $ hg cp bar xyzzy
xyzzy: not overwriting - file exists xyzzy: not overwriting - file exists
('hg copy --after' to record the copy) ('hg copy --after' to record the copy)
[1]
$ cd .. $ cd ..

tests/test-rename.t

$ hg update -C $ hg update -C
1 files updated, 0 files merged, 0 files removed, 0 files unresolved 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ rm d2/c $ rm d2/c
rename --after a single file to a nonexistent target filename rename --after a single file to a nonexistent target filename
$ hg rename --after d1/a dummy $ hg rename --after d1/a dummy
d1/a: not recording move - dummy does not exist d1/a: not recording move - dummy does not exist
[1]
move a single file to an existing directory move a single file to an existing directory
$ hg rename d1/d11/a1 d2 $ hg rename d1/d11/a1 d2
$ hg status -C $ hg status -C
A d2/a1 A d2/a1
d1/d11/a1 d1/d11/a1
R d1/d11/a1 R d1/d11/a1
move everything under directory d1 to existing directory d2, do not move everything under directory d1 to existing directory d2, do not
overwrite existing files (d2/b) overwrite existing files (d2/b)
$ hg rename d1/* d2 $ hg rename d1/* d2
d2/b: not overwriting - file already committed d2/b: not overwriting - file already committed
('hg rename --force' to replace the file by recording a rename) ('hg rename --force' to replace the file by recording a rename)
moving d1/d11/a1 to d2/d11/a1 moving d1/d11/a1 to d2/d11/a1
[1]
$ hg status -C $ hg status -C
A d2/a A d2/a
d1/a d1/a
A d2/ba A d2/ba
d1/ba d1/ba
A d2/d11/a1 A d2/d11/a1
d1/d11/a1 d1/d11/a1
R d1/a R d1/a
$ mkdir d2/d21 $ mkdir d2/d21
$ mv d1/a d1/d11/a1 d2/d21 $ mv d1/a d1/d11/a1 d2/d21
$ hg rename --after 'glob:d1/**' d2/d21 $ hg rename --after 'glob:d1/**' d2/d21
moving d1/a to d2/d21/a moving d1/a to d2/d21/a
d1/b: not recording move - d2/d21/b does not exist d1/b: not recording move - d2/d21/b does not exist
d1/ba: not recording move - d2/d21/ba does not exist d1/ba: not recording move - d2/d21/ba does not exist
moving d1/d11/a1 to d2/d21/a1 moving d1/d11/a1 to d2/d21/a1
[1]
pulkitUnsubmitted
Not Done

Not sure how I feel about this one. There are few operations which succeed and few which does not.

pulkit: Not sure how I feel about this one. There are few operations which succeed and few which does…
$ hg status -C $ hg status -C
A d2/d21/a A d2/d21/a
d1/a d1/a
A d2/d21/a1 A d2/d21/a1
d1/d11/a1 d1/d11/a1
R d1/a R d1/a
R d1/d11/a1 R d1/d11/a1
$ hg update -C $ hg update -C
$ rm -rf d2/d21 $ rm -rf d2/d21
attempt to overwrite an existing file attempt to overwrite an existing file
$ echo "ca" > d1/ca $ echo "ca" > d1/ca
$ hg rename d1/ba d1/ca $ hg rename d1/ba d1/ca
d1/ca: not overwriting - file exists d1/ca: not overwriting - file exists
('hg rename --after' to record the rename) ('hg rename --after' to record the rename)
[1]
$ hg status -C $ hg status -C
? d1/ca ? d1/ca
$ hg update -C $ hg update -C
0 files updated, 0 files merged, 0 files removed, 0 files unresolved 0 files updated, 0 files merged, 0 files removed, 0 files unresolved
forced overwrite of an existing file forced overwrite of an existing file
$ echo "ca" > d1/ca $ echo "ca" > d1/ca
$ hg rename --force d1/ba d1/ca $ hg rename --force d1/ba d1/ca
$ hg status -C $ hg status -C
A d1/ca A d1/ca
d1/ba d1/ba
R d1/ba R d1/ba
$ hg update -C $ hg update -C
1 files updated, 0 files merged, 0 files removed, 0 files unresolved 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ rm d1/ca $ rm d1/ca
attempt to overwrite an existing broken symlink attempt to overwrite an existing broken symlink
#if symlink #if symlink
$ ln -s ba d1/ca $ ln -s ba d1/ca
$ hg rename --traceback d1/ba d1/ca $ hg rename --traceback d1/ba d1/ca
d1/ca: not overwriting - file exists d1/ca: not overwriting - file exists
('hg rename --after' to record the rename) ('hg rename --after' to record the rename)
[1]
$ hg status -C $ hg status -C
? d1/ca ? d1/ca
$ hg update -C $ hg update -C
0 files updated, 0 files merged, 0 files removed, 0 files unresolved 0 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ rm d1/ca $ rm d1/ca
replace a symlink with a file replace a symlink with a file
#endif #endif
do not copy more than one source file to the same destination file do not copy more than one source file to the same destination file
$ mkdir d3 $ mkdir d3
$ hg rename d1/* d2/* d3 $ hg rename d1/* d2/* d3
moving d1/d11/a1 to d3/d11/a1 moving d1/d11/a1 to d3/d11/a1
d3/b: not overwriting - d2/b collides with d1/b d3/b: not overwriting - d2/b collides with d1/b
[1]
$ hg status -C $ hg status -C
A d3/a A d3/a
d1/a d1/a
A d3/b A d3/b
d1/b d1/b
A d3/ba A d3/ba
d1/ba d1/ba
A d3/d11/a1 A d3/d11/a1