classification
Title: move_file()'s return value when dry_run=1 unclear
Type: behavior Stage: resolved
Components: Distutils Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: ajaksu2, eelis, eric.araujo, steve.dower, tarek
Priority: normal Keywords: patch

Created on 2005-01-31 05:18 by eelis, last changed 2021-02-03 18:37 by steve.dower. This issue is now closed.

Files
File name Uploaded Description Edit
move_file_dry_run.diff ajaksu2, 2009-02-09 23:57 Change dry_run from "exit early" to "exit when name matches real move target"
Messages (6)
msg60643 - (view) Author: Eelis (eelis) Date: 2005-01-31 05:18
distutils.file_util.move_file has the following signature:

  move_file(src, dst, verbose=0, dry_run=0)

Unfortunately, the purpose of the dry_run parameter is
not described in the documentation. However, it's
obvious that one effect of specifying dry_run=1 should
be that the file is not actually moved. Less obvious is
how dry_run=1 affects move_file's return value, which
the documentation describes as follows:

  "Returns the new full name of the file."

Should the dry_run parameter be allowed to affect the
return value? My guess is that it shouldn't, but it
does (on win32 and linux):

  # given a file named foo and a directory named bar
  move_file('foo', 'bar', 0, 1)  #-> 'bar'
  move_file('foo', 'bar', 0, 0)  #-> 'bar/foo'

A quick look at the source code showed that:

  if isdir(dst):
      dst = os.path.join(dst, basename(src))

is executed _before_:

    if dry_run:
        return dst

which causes the discrepancy.

Is this a bug, or should the return value of move_file
when dry_run=1 be considered unreliable? If the latter
is the case, perhaps it should be documented explicitly.
msg81509 - (view) Author: Daniel Diniz (ajaksu2) (Python triager) Date: 2009-02-09 22:25
Confirmed for trunk and py3k. Might look unimportant, but IMHO having
the same results with dry_run=1 would make it much easier to e.g.
generate target lists.

Let me know if a patch would help.
msg81510 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-02-09 22:30
> Let me know if a patch would help.

Sure !
msg81517 - (view) Author: Daniel Diniz (ajaksu2) (Python triager) Date: 2009-02-09 23:57
Here's a very simple-minded patch (with microtest) that changes behavior
in a questionable way.

If it goes in as-is, dry_run=1 will not always succeed anymore. So it'd
be incompatible... but a 1:1 representation of a real move kinda
requires some failure mode.

It's possible to avoid raising when dry_run=1, logging a warning that a
real move would fail, and return the imaginary new full name. Or return
None, '', etc. Suggestions?

Perhaps studying a use-case makes things clearer. 

Looks like copy_file already has the same return value independent of
dry_run.
msg154191 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-25 06:50
My opinion about dry-run modes is that the goal is to be as close as possible to the real operation, but leave the world in a clean state.  Therefore, testing for file existence is okay (nevermind about modifying the last accessed time, it isn’t important IMO), but creating a file is not okay.  So I’m perfectly fine if calling the function with dry_run=1 will now not always succeed.  You want to see that error when running in dry-run mode.

I’ll improve the test and commit the patch.

packaging uses shutil.move directly, which does not return anything; I will check if packaging does not need it or if we have a regression and need to improve shutil.move in 3.3.
msg386447 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2021-02-03 18:37
Distutils is now deprecated (see PEP 632) and all tagged issues are being closed. From now until removal, only release blocking issues will be considered for distutils.

If this issue does not relate to distutils, please remove the component and reopen it. If you believe it still requires a fix, most likely the issue should be re-reported at https://github.com/pypa/setuptools
History
Date User Action Args
2021-02-03 18:37:43steve.dowersetstatus: open -> closed

nosy: + steve.dower
messages: + msg386447

resolution: out of date
stage: patch review -> resolved
2012-02-25 06:51:00eric.araujosetassignee: tarek -> eric.araujo
messages: + msg154191
2012-02-24 10:47:50ezio.melottisetnosy: + eric.araujo

versions: + Python 3.3, - Python 3.1
2010-08-19 19:00:20BreamoreBoysetstage: patch review
versions: + Python 3.2
2009-02-09 23:57:47ajaksu2setfiles: + move_file_dry_run.diff
keywords: + patch
messages: + msg81517
2009-02-09 22:30:22tareksetassignee: tarek
messages: + msg81510
2009-02-09 22:25:57ajaksu2setnosy: + ajaksu2, tarek
type: behavior
messages: + msg81509
versions: + Python 3.1, Python 2.7
2005-01-31 05:18:41eeliscreate