Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix distutils bdist crash #261

Merged

Conversation

lunkwill42
Copy link
Contributor

The build_sass command adds the list of compiled css files to the distribution's data_files attribute.

However, it does not add entries that are according to distutils' spec. This causes commands like python setup.py bdist or pip install to crash.

According to the docs, an entry in data_files should either be a single string, or a two-tuple of (dir_name, [list_of_files]). The existing code instead adds two-tuples of (dir_name, file_name) for each file, causing distutils to attempt to iterate over the characters of the file_name string and crash when it attempts to copy a non-existent, one-character-named file.

I'm not entirely sure how to write an effective test for this (and there doesn't seem to be an existing one to cover this code path).

An example of a typical traceback from running bdist with then environment variable DISTUTILS_DEBUG=1:

running install_data
Distribution.get_command_obj(): creating 'install_data' command object
Traceback (most recent call last):
  File "setup.py", line 64, in <module>
    zip_safe=False,
  File "/source/fjas/local/lib/python2.7/site-packages/setuptools/__init__.py", line 140, in setup
    return distutils.core.setup(**attrs)
  File "/usr/lib/python2.7/distutils/core.py", line 151, in setup
    dist.run_commands()
  File "/usr/lib/python2.7/distutils/dist.py", line 953, in run_commands
    self.run_command(cmd)
  File "/usr/lib/python2.7/distutils/dist.py", line 972, in run_command
    cmd_obj.run()
  File "/usr/lib/python2.7/distutils/command/bdist.py", line 146, in run
    self.run_command(cmd_name)
  File "/usr/lib/python2.7/distutils/cmd.py", line 326, in run_command
    self.distribution.run_command(command)
  File "/usr/lib/python2.7/distutils/dist.py", line 972, in run_command
    cmd_obj.run()
  File "/usr/lib/python2.7/distutils/command/bdist_dumb.py", line 94, in run
    self.run_command('install')
  File "/usr/lib/python2.7/distutils/cmd.py", line 326, in run_command
    self.distribution.run_command(command)
  File "/usr/lib/python2.7/distutils/dist.py", line 972, in run_command
    cmd_obj.run()
  File "/source/fjas/local/lib/python2.7/site-packages/setuptools/command/install.py", line 61, in run
    return orig.install.run(self)
  File "/usr/lib/python2.7/distutils/command/install.py", line 613, in run
    self.run_command(cmd_name)
  File "/usr/lib/python2.7/distutils/cmd.py", line 326, in run_command
    self.distribution.run_command(command)
  File "/usr/lib/python2.7/distutils/dist.py", line 972, in run_command
    cmd_obj.run()
  File "/usr/lib/python2.7/distutils/command/install_data.py", line 74, in run
    (out, _) = self.copy_file(data, dir)
  File "/usr/lib/python2.7/distutils/cmd.py", line 365, in copy_file
    dry_run=self.dry_run)
  File "/usr/lib/python2.7/distutils/file_util.py", line 110, in copy_file
    "can't copy '%s': doesn't exist or not a regular file" % src)
distutils.errors.DistutilsFileError: can't copy 's': doesn't exist or not a regular file

Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Can you rebase this to reapply formatting? (I just merged this on another branch)

# rebase or merge (not shown) 
pre-commit run --all-files 
# then commit those changes (not shown) 

…s data files manifest

According to the docs, an entry in data_files should either be a single string,
or a two-tuple of (dir_name, [list_of_files]).
@lunkwill42 lunkwill42 force-pushed the distutils-correct-data-files-structure branch from 4f70e5e to d8428ee Compare August 27, 2018 07:21
@lunkwill42
Copy link
Contributor Author

Actually, I ended up melding my commit and the pre-commit formatting changes into a single commit, but I guess the end result is the same :)

Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even better!

@asottile asottile merged commit a0308b0 into sass:master Aug 27, 2018
@asottile
Copy link
Member

Thanks!

@lunkwill42
Copy link
Contributor Author

No problem, hope to see a release on PyPI soon :-)

@lunkwill42 lunkwill42 deleted the distutils-correct-data-files-structure branch August 27, 2018 13:48
@asottile
Copy link
Member

Yep! plan to get in one more change for #134 and then release 0.15.0

asottile added a commit that referenced this pull request Sep 24, 2018
Fix sdist with data files change in #261
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants