set root_dir do not work

output:
=====================================================

Python 3.4.0 (v3.4.0:04f714765c13, Mar 16 2014, 19:25:23) [MSC v.1600 64 bit (AMD64)] on win32
Type "copyright", "credits" or "license()" for more information.
>>> import shutil
>>> shutil.make_archive("tmp.tar.gz", "gztar", "c:/xjtu", "c:/tmp")
'C:\\Python34\\tmp.tar.gz.tar.gz'

=====================================================

source code of make_archive()
=====================================================
756 save_cwd = os.getcwd()
757 if root_dir is not None:
758 if logger is not None:
759 logger.debug("changing into '%s'", root_dir)
760 base_name = os.path.abspath(base_name)
761 if not dry_run:
762 os.chdir(root_dir)
...
...
782 try:
783 filename = func(base_name, base_dir, **kwargs)
784 finally:
=====================================================

base_name is set before chdir, so the archive always be created in cwd, whether set root_dir or not.
so, line 760 should be move below line 762

Thanks for the report, do you want to provide a patch?
(You can check the devguide if you need more information.)

I believe this is working as designed, although the documentation does not make that clear. root dir is the root directory of the *created archive*, it has nothing to do with where the archive file itself is placed.

I don't think so.

In source code, it just change work dir to root_dir, do nothing, and then the change word dir back.

If it works as design, the "root_dir" will be meaningless. should be remove.

Here's the path

The point *should* be that if you have something like:

/home/me/some/directory/my/stuff/a
/home/me/some/directory/my/stuff/b
/home/me/some/other/directory

and you set rootdir to '/home/me/some' and base_dir='/home/me/some/directory/my' then the file paths in the archive will be:

directory/my/stuff/a
directory/my/stuff/b

At least, that's how I read the docs, though as I said they are *not* clear. (I can't otherwise imagine any reason to have the root_dir parameter, with that name.)

Is this not what happens?

that sounds reasonable

Sorry to bother.

But This patch is still not accepted.

I still suffer this issue.

Is anyone willing to review this patch?

Thanks

Or, Is there anything I could help?

I am glad to help fixing this issue.

Hi bananaapple, there are two things that need to be addressed:

1. We need to decide whether this is a bug in make_archive() implementation or not. See msg223572 and msg223668 for details. I personally agree with David's analysis in msg223668.

We can rephrase make_archive() documentation to clarify what root_dir exactly does.

2. If this is a bug in make_archive() implementation, we need to add a test case. You can take a look at Lib/test/test_shutil.py.

Agree with David, this is working as designed (and matching the behavior of say "tar -C"). But the documentation is not clear if not confusing.

Hello Berker Peksag,

1. For now, I realize the comment of David.

This is not a bug of make_archive() implementation.

However, I think documentation is not clear to understand at the first time reading it.

Maybe it should be rephrased.

2. Actually, what I am looking in this function is able to output the archive to another directory rather than current directory.

But, this functionality is not provided.

I think this functionality will be useful if implemented.

What do you think about this?

Thank you for your explaination.

"base_name is the name of the file to create, including the path". So you should be able to specify the path to where you want it created. If that doesn't work, that's a bug.

I am sorry.

I understand now.

Thank you.

I'm similarly confused by this issue. If somebody can help me understand what's going on, I'll put my understanding into a documentation patch.

I have created this minimal example to demonstrate what I don't understand: I've created a directory structure within /tmp like this:

bpo-22021
└── root
└── structure
├── content
│   └── please_add.txt
└── do_not_add.txt

Now I'd like to create a zip archive that contains the directories "structure" and "content", and the file "please_add.txt", but not the file "do_not_add.txt". My understanding of the documentation was that I'd have to do this:

>> shutil.make_archive(base_name='/tmp/issue22021archive', format='zip', root_dir='/tmp/issue22021/root', base_dir='/tmp/issue22021/root/structure/content')

But on my system, the created file (/tmp/issue22021archive.zip) looks like this according to unzip -l:

Archive: issue22021archive.zip
Length Date Time Name
--------- ---------- ----- ----
0 2018-05-30 00:26 tmp/issue22021/root/structure/content/
0 2018-05-30 00:26 tmp/issue22021/root/structure/content/please_add.txt
--------- -------
0 2 files

This is consistent with my experience that created archives will always contain directories from / on, which is unexpected to me.

It appears this happens whenever base_dir and root_dir is set, but to my understanding the documentation does not warn against this.

I've confirmed this behaviour with Python 3.6.5 and Python 3.5.3, but I suspect that doesn't really matter, as it seems to be a documentation issue.

I think this snippet might help you.

shutil.make_archive(base_name='/tmp/issue22021archive', format='zip', root_dir='/tmp/issue22021/root/', base_dir='structure/content/')

unzip -l /tmp/issue22021archive.zip

Archive: /tmp/issue22021archive.zip
Length Date Time Name
--------- ---------- ----- ----
0 2018-05-30 11:14 structure/content/
0 2018-05-30 11:02 structure/content/please_add.txt
--------- -------
0 2 files

Thank you, that's what I figured out later last evening. To my understanding, the docs don't give any indication that base_dir is supposed to be relative to root_dir, so I'd add this information, and maybe add a similar example to the one above, if that's appropriate?

Sounds reasonable to me.

FYI.

Joannah Nanjekye converted bpo-22021.patch into a PR: PR 10191.

Serhiy Storchaka closed the PR: "The current behavior looks correct to me. It is consistent with the behavior of the tar command. This change breaks tests and I think it will break user code. Read the discussion on bpo-22021."
#10191 (comment)

This is a documentation issue.

Is anybody working on this or can I submit a PR?

Is anybody working on this or can I submit a PR?

Yes: Joannah Nanjekye wrote PR 10191 (from bpo-22021.patch) which has been rejected. This issue is more complex from what it looks.

Since Serhiy said that this is a pure documentation issue, I thought that a doc PR is all that was needed, which I would be happy to make. Am I missing something here?

Yes, this is a documentation issue: A patch clarifying what root_dir and base_dir do, and how they interact (or how they are to be used in combination) would be sufficient to close this issue.

Ping.

Pinging once more for review.

@giampaolo.rodola, would you be able to provide a review of this doc change? Thanks!

@cheryl.sabella don't have the bandwidth right now (traveling), sorry.

@giampaolo.rodola, no problem. I intended this more as a general request rather than something for right now. If you do have any availability at some point, please keep this one in mind. Thanks!

New changeset 7633371 by Lysandros Nikolaou in branch 'master':
bpo-22021: Update root_dir and base_dir documentation in shutil (GH-10367)
7633371

New changeset d5489a9 by Miss Islington (bot) in branch '3.7':
bpo-22021: Update root_dir and base_dir documentation in shutil (GH-10367)
d5489a9

New changeset be5ed59 by Miss Islington (bot) in branch '3.9':
bpo-22021: Update root_dir and base_dir documentation in shutil (GH-10367)
be5ed59

New changeset 12dfbae by Miss Islington (bot) in branch '3.8':
bpo-22021: Update root_dir and base_dir documentation in shutil (GH-10367)
12dfbae

Since #54576 is now merged, should we close this?