flit
flit copied to clipboard
convert git's path separators to match os.sep
git returns file paths separated by / regardless of the platform.
After flit.build.main() is called, starting from this line:
flit.vcs.git.list_untracked_deleted_files()returns a list of files in the directory, all using/as the path separatorflit.sdist.include_path()checks if this string starts withdist\on Windows- A
VCSErroris thrown because thedist/folder is not excluded
The current tests fake listing the files in the directory, but do not list the dist/module1-0.1.tar.gz that flit creates as untracked.
I changed the mock git script to behave a little more similarly to the git commands that would be called.
The simpler fix would be to have the mock git script return dist/module1-0.1.tar.gz when run with --deleted, but this way each test can specify the tracked files, and the files produced by flit are dynamically discovered.
I can understand if this makes the tests too flaky.
@mawillcockson do you think you'll get back to this, or would you like me to take it over?
@mawillcockson do you think you'll get back to this, or would you like me to take it over?
Yes, I can finish it, sorry for the delay.
I went down a rabbit hole trying to find out if os.fsdecode() is necessary, or if git will return UTF-8 regardless of the filesystem encoding, and os.fsdecode() may not decode non-ASCII text correctly:
https://github.com/takluyver/flit/blob/353f920b7a1a95c28ba1234d275324c351d6f4a3/flit/vcs/git.py#L6-L9
I wasn't able to come to a conclusion, so I'm leaving it as-is with a note, in case someone finds a bug with it.
I don't think we have any tests where this will actually find 'untracked' files at the moment, so this extra complexity isn't actually helping us test anything. Are you planning to add such a test?
Apologies: I abandoned this PR for so long, I forgot most of the context.
None of the tests currently find the untracked files, because the original mock script was not designed to print any.
For example, if git were real in the test_build_main test, this section of flit would be run:
https://github.com/pypa/flit/blob/9335bfbf23bf79b05b30c23e909c073a477e7f65/flit_core/flit_core/sdist.py#L159-L171
- Line 160 creates the
distdirectory - Lines 166-168 make a
module1-0.1.tar.gzfile in that directory - Line 171 eventually runs both:
git ls-files --deleted --others --exclude-standard -zwhich will returndist/module1-0.1.tar.gzwith a forward slash, regardless of the platformgit ls-files --recurse-submodules -zwhich returns the files that are part ofmodule1_toml:EG-README.rst,module1.py, andpyproject.toml
The original script simulated only the git ls-files --recurse-submodules -z call.
The more complicated rewrite of the script I added and then subsequently removed did both.
In light of #445 adding tests with real git, I agree that my script was too complex.
I think adding a helper function could minimize the added complexity?
I also added a test that explicitly runs the build step twice. I think it's possible for the build steps outlined above to change, and running it twice ensures the second build will interact with the previous build's artifacts (e.g. the dist/ folder).
I have confirmed that, without the addition of os.path.normpath(), the tests fail on Windows, and pass on Linux.
I've rebased to include the namespace subpackage test.