Missing files in the tarball

[thmo]

Please include

README.textile
LICENSE.txt

in the tarball. For building the Fedora package, we have to separately download them.

It might also be good to have the tests dir as part of the tarball, so we can run the testsuite while creating the RPM.

[sebix]

Regarding the files: True, and also the changelog and contributors files are missing in the pip-package. Not sure about the todo.

It might also be good to have the tests dir as part of the tarball, so we can run the testsuite while creating the RPM.

We could move it to textile and make it a submodule, would this be ok for you?

[thmo]

Not sure about the todo.

That's probably not needed. We usually don't include it in the "binary" RPM.

It might also be good to have the tests dir as part of the tarball, so we can run the testsuite while creating the RPM.

We could move it to textile and make it a submodule, would this be ok for you?

On this I am a bit undecided. http://doc.pytest.org/en/latest/goodpractices.html#choosing-a-test-layout-import-rules shows both layouts (inline and extra dir). From an RPM packager pov, I'd say the tests should not be installed system-wide (as they are intended for developers), so I'd favor the extra dir layout (which you currently use). I am not sure however, how to trigger its inclusion for sdist in setup.py, maybe via MANIFEST.in listing tests, pytest.ini, etc.

[sebix]

You could always do a rm -r %{buildroot}%{python3_sitelib}/textile/tests/ in your specfile :D

Please test it: https://testpypi.python.org/pypi/textile/2.3.3
The tests/ dir is included in the source tarball, but not in the wheel. If it's correct, I will push the used MANIFEST.in.

I'm not sure if we should do a new release because of the last 3 fixes, which all affected only the packaging. But I can't upload to the real pypi anyway.

[thmo]

Please test it: https://testpypi.python.org/pypi/textile/2.3.3
The tests/ dir is included in the source tarball, but not in the wheel. If it's correct, I will push the used MANIFEST.in.

Looks good to me.

Two questions:

• I see three failures in test_imagesize despite PIL being installed - should I create a separate issue for this?
• setup.py specifies a dependency on pytest-cov although a coverage report is only given when explicitly requested via py.test --cov - should the dependency be dropped?

As a side note, .coveragerc is also not in the tarball (and might itself be updated, as it has omit = textile/tests/* in it.)

[ikirudennis]

@sebix please submit a pull request for those changes. It sounds like they should be trivial to merge in and make a release for them. Thanks

@thmo Yes, please create a separate issue for those failures. And the pytest.ini specifies the --cov argument as a default option. It's still Sunday morning here, and I'm not sure my brain is at full speed yet: Is pytest.ini being excluded from the sdist? It might need to be included along with .coveragerc.

[sebix]

Okay, I'll include the .coveragerc too.

[thmo]

@ikirudennis For some reason I currently don't understand, the failures don't show up anymore.

@sebix This tarball does neither contain a pytest.ini nor a .coveragerc.

[sebix]

@thmo Fixed with a new file: https://testpypi.python.org/pypi/textile/ (the 2.3.3-1)

[ikirudennis]

@thmo, we've all been there before, right? If @sebix's file works for you, I'll merge it, and push it out to pypi.

[thmo]

The updated tarfile works for me, although it contains additional, different changes - for example it installs a /usr/bin/textile binary.

This binary is welcomed in principle, but - at least for Fedora - there's a name clash. /usr/bin/textile is already provided by the perl-Text-Textile package, so I'd have to rename it to something else. e.g. /usr/bin/pytextile. Suggestions?

[sebix]

This command has been added a while ago in 0be616b

concerning the clash: What about providing e.g. /usr/bin/py(thon-)textile(-3.5) as default and /usr/bin/textile via update-alternatives?

[thmo]

This command has been added a while ago in 0be616b

That's on branch develop, and has not been merged to master. Thus it's also not present in 2.3.3.

[sebix]

That's on branch develop, and has not been merged to master. Thus it's also not present in 2.3.3.

Yes, but develop is the next release. Thus it will be in 2.3.4

[ikirudennis]

I had a feeling some sort of name collision would happen with the command line tool. I'm fine with changing it to pytextile. Would we need to change it in the setup.py, or are you able to change it as @sebix asked? (I'm not terribly familiar with packaging for Fedora, so please forgive my ignorance.)

[thmo]

If you change it in the setup.py, it's slightly easier for me, but more importantly, there's a chance that the name will be the same across distros. One of the Fedora principles is to get as much as possible upstream.

That said, renaming it as part of the packaging would also be no big deal.

Adding the update-alternatives machinery is a bit more work and would require coordination with the packager of the perl package.

[sebix]

Fixed in fa20408

[mitya57]

textile-2.3.5.tar.gz still does not include tests, despite what the changelog says.

[ikirudennis]

I just uploaded version 2.3.6 which now has the tests directory.

[mitya57]

Ok, thanks, will be useful for me in Debian.

[sebix]

9998e8e added the tests-directory, available in release 2.3.6. But it's not in in 2.3.7 for some reasons.

[ikirudennis]

Hmm, this is strange. I'm not sure why, but setup.py is being weird. I ran it as is, and it didn't include it. I had to change it to include tests/* and it included it. I reverted the change, removed the dist directory, and re-ran it and it was included. So, I'll remember to keep an eye on it whenever I create a new release. Thanks for bringing it to my attention.

[sebix]

How do you create the source tarballs? Running python setup.py sdist locally includes the tests/fixtures, while the archive on pypi does not have it. But the archive there has a lot of temporary files such as tests/.test_github_issues.py.un~.

[sebix]

And where do the files textile/rawlink_textile.py and textile/plaintext.py come from?

[ikirudennis]

Yeah, I run python setup.py sdist bdist_wheel. Building both of them simultaneously shouldn't affect either build, right? I'll do some experimenting.

That's weird about rawlink and plaintext. I had no idea they would be included in the zip.

Thanks for the eagle eyes on this. I'll see about cleaning it up and re-upload.

[thmo]

Hm, these two files now ended up in the 2.3.8 Fedora RPMs. Hope this doesn't create confusion for someone...

Please do not upload a modfiied zip file with the same name, that's bad style. Either add some postfix to, or simply increase the version number.

[sebix]

Hm, these two files now ended up in the 2.3.8 Fedora RPMs. Hope this doesn't create confusion for someone...

I also noticed it during packaging (for openSUSE) because rpmlint complained about the shebangs :) But the existence of these files shouldn't cause problems.

[thmo]

I also noticed it during packaging (for openSUSE) because rpmlint complained about the shebangs :)

In the Fedora spec file, I always remove the shebangs from all .py files anyway, so I didn't notice ;)

[sebix]

In 2.3.9 the two files are gone now, but tests/fixtures/README.txt is still missing.

[ikirudennis]

I gotta say, this whole MANIFEST and sdist thing has me pretty baffled. It's not consistent across machines or even consecutive runs on the same machine.

I'll see what I can cook up.

[ikirudennis]

So, I decided to start from scratch with MANIFEST.in. And it turns out without it, the packaging is pretty smart without a manifest. It just needed an entry for test fixture, and the manifest file itself in order to include them in the source distribution. The icing on the cake is that it turns out that those rawlink and plaintext files which were mistakenly included in the source, have actually been included in the wheels of previous versions as well. It's looking like 2.3.10 will be a bit slimmer than previous versions.

Thanks again, @sebix and @thmo for your attention to detail.