Repository: pypa/setuptools
Pull request #1481 (2018-09-14 14:04:19)

build_meta sdist directory delegate to --dist-dir
This avoids a lot of current bugs (e.g. sdists are always generated in ``dist``, and at end moved out; however ``dist`` folder remains around - so if e.g. the version changes you'll get two sdists now); no need to move around just build in the requested location. Discovered this while trying to use this backend with tox's PEP-517/8.

pganssle: I think the code coverage failure is just because you have simplified the code and the total number of covered lines went down, so I think we can ignore it.
pganssle: @gaborbernat Can you add some tests for any of the bugs that this fixes?
gaborbernat: I can try :+1: there are multiple bugs though.
pganssle: @gaborbernat "This fixes too many bugs" is a pretty nice problem to have. ;)
benoit-pierre: @pganssle: what's the deal with netlify? I can't reproduce the issue locally when running `tox -e docs` with Python 2.7.15.
gaborbernat: @pganssle added two of them
pganssle: @gaborbernat I rebased this against master because I believe the netlify problem is fixed in master, but that obviously wiped out your signatures. [Here's my alias](https://github.com/pganssle/dotfiles/blob/master/dotfiles/git/.gitconfig#L12) for "rebase-and-sign-all-commits" if you want to use it to do a new rebase.

Patch:
diff --git a/changelog.d/1481.change.rst b/changelog.d/1481.change.rst
new file mode 100644
index 000000000..463ba6d92
--- /dev/null
+++ b/changelog.d/1481.change.rst
@@ -0,0 +1,1 @@
+Join the sdist ``--dist-dir`` and the ``build_meta`` sdist directory argument to point to the same target (meaning the build frontend no longer needs to clean manually the dist dir to avoid multiple sdist presence, and setuptools no longer needs to handle conflicts between the two).
diff --git a/setuptools/build_meta.py b/setuptools/build_meta.py
index f7f9bda24..0067a7ac3 100644
--- a/setuptools/build_meta.py
+++ b/setuptools/build_meta.py
@@ -171,11 +171,9 @@ def build_sdist(sdist_directory, config_settings=None):
     config_settings = _fix_config(config_settings)
     sdist_directory = os.path.abspath(sdist_directory)
     sys.argv = sys.argv[:1] + ['sdist'] + \
-        config_settings["--global-option"]
+        config_settings["--global-option"] + \
+        ["--dist-dir", sdist_directory]
     _run_setup()
-    if sdist_directory != 'dist':
-        shutil.rmtree(sdist_directory)
-        shutil.copytree('dist', sdist_directory)
 
     sdists = [f for f in os.listdir(sdist_directory)
               if f.endswith('.tar.gz')]


Test patch:
diff --git a/setuptools/tests/test_build_meta.py b/setuptools/tests/test_build_meta.py
index f1d517bb0..7b195e2c3 100644
--- a/setuptools/tests/test_build_meta.py
+++ b/setuptools/tests/test_build_meta.py
@@ -1,6 +1,7 @@
 from __future__ import unicode_literals
 
 import os
+import shutil
 
 import pytest
 
@@ -9,7 +10,6 @@ from .textwrap import DALS
 
 __metaclass__ = type
 
-
 futures = pytest.importorskip('concurrent.futures')
 importlib = pytest.importorskip('importlib')
 
@@ -23,12 +23,14 @@ class BuildBackendBase:
 
 class BuildBackend(BuildBackendBase):
     """PEP 517 Build Backend"""
+
     def __init__(self, *args, **kwargs):
         super(BuildBackend, self).__init__(*args, **kwargs)
         self.pool = futures.ProcessPoolExecutor()
 
     def __getattr__(self, name):
         """Handles aribrary function invocations on the build backend."""
+
         def method(*args, **kw):
             root = os.path.abspath(self.cwd)
             caller = BuildBackendCaller(root, self.env, self.backend_name)
@@ -51,6 +53,7 @@ defns = [
         'setup.py': DALS("""
             __import__('setuptools').setup(
                 name='foo',
+                version='0.0.0',
                 py_modules=['hello'],
                 setup_requires=['six'],
             )
@@ -65,6 +68,7 @@ defns = [
             assert __name__ == '__main__'
             __import__('setuptools').setup(
                 name='foo',
+                version='0.0.0',
                 py_modules=['hello'],
                 setup_requires=['six'],
             )
@@ -82,6 +86,7 @@ defns = [
             assert variable
             __import__('setuptools').setup(
                 name='foo',
+                version='0.0.0',
                 py_modules=['hello'],
                 setup_requires=['six'],
             )
@@ -146,3 +151,32 @@ def test_prepare_metadata_for_build_wheel_with_str(build_backend):
     dist_info = build_backend.prepare_metadata_for_build_wheel(dist_dir)
 
     assert os.path.isfile(os.path.join(dist_dir, dist_info, 'METADATA'))
+
+
+def test_build_sdist_explicit_dist(build_backend):
+    # explicitly specifying the dist folder should work
+    # the folder sdist_directory and the ``--dist-dir`` can be the same
+    dist_dir = os.path.abspath('dist')
+    sdist_name = build_backend.build_sdist(dist_dir)
+    assert os.path.isfile(os.path.join(dist_dir, sdist_name))
+
+
+def test_build_sdist_version_change(build_backend):
+    sdist_into_directory = os.path.abspath("out_sdist")
+    os.makedirs(sdist_into_directory)
+
+    sdist_name = build_backend.build_sdist(sdist_into_directory)
+    assert os.path.isfile(os.path.join(sdist_into_directory, sdist_name))
+
+    # if the setup.py changes subsequent call of the build meta should still succeed, given the
+    # sdist_directory the frontend specifies is empty
+    with open(os.path.abspath("setup.py"), 'rt') as file_handler:
+        content = file_handler.read()
+    with open(os.path.abspath("setup.py"), 'wt') as file_handler:
+        file_handler.write(content.replace("version='0.0.0'", "version='0.0.1'"))
+
+    shutil.rmtree(sdist_into_directory)
+    os.makedirs(sdist_into_directory)
+
+    sdist_name = build_backend.build_sdist("out_sdist")
+    assert os.path.isfile(os.path.join(os.path.abspath("out_sdist"), sdist_name))