-
Notifications
You must be signed in to change notification settings - Fork 30
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
Always use the ustar format of tar #73
base: master
Are you sure you want to change the base?
Conversation
This should prevent the module from being uninstallable on systems that don't support PAX headers, but the tar of the creator of the distribution supports them. See e.g. Dual-Life/Devel-PPPort#183 or https://huntingbears.nl/2015/02/17/new-tar-paxheaders-and-installing-from-cpan/
Is this portable? |
I have no idea :-( but it probably isn't |
It looks like that GNU tar and BSD tar supports |
lib/Module/Starter/Simple.pm
Outdated
@@ -440,6 +440,7 @@ my %WriteMakefileArgs = ( | |||
#'ABC' => '1.6', | |||
#'Foo::Bar::Module' => '5.0401', | |||
}, | |||
macro => { TARFLAGS => '--format=ustar -c -v -f' }, | |||
dist => { COMPRESS => 'gzip -9f', SUFFIX => 'gz', }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to ExtUtils::MakeMaker documentation, TARFLAGS
should be in dist
key, not in macro
key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, it seems to work both ways. Also, I'm not sure what I originally read about BSD, it seems its tar supports --format ustar
as well as -o
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dist
section is options for creating the distribution tarball which is what we want here. macro
is undocumented so we would need an archaeologist to find its purpose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
macro
is general purpose, dist
is for building tarballs specifically. macro
happens to work because it happens to be defined after dist, but this shouldn't be relied upon.
Normally, tar on MacOS won't use PAX headers. However, if some file attributes aren't possible to represent in a standard ustar format, it will add the PaxHeaders. This happens on one of my machines due to having a uid and gid that are too large. However, specifying I'm not certain what happened to cause the PAX headers in the Devel-PPPort case. |
Updated with a fixup. Maybe we can first run |
Remember that this is not Module::Starter's Makefile.PL, it is a template for any distribution created with Module::Starter, that is then expected to be maintained by the author. I am not in favor of adding anything complex. |
@Grinnz I know. I wanted to prevent new distributions from using PAX headers and failing to install randomly. |
Seems that GNU tar On the other hand BSD tar So to enforce ustar on both GNU tar and BSD tar, it is needed to use |
In case my previous comment wasn't clear: specifying the format will not cause bsd tar (at least on MacOS) to create correct archives where it was previously creating bad archives. Instead, it will cause it to create empty archives. |
What about switching to
|
@haarg: Does |
Archive::Tar/ptar will create a normal archive. |
A solution requiring 5.10 is tricky. Even if I set the initial minimum to 5.10 (I would consider 5.8), there is nothing keeping the author from then lowering it, and having a broken dist. |
That module is in Core since 5.10. But it does not mean you cannot install it also on previous version. Also it is needed only when creating new distribution release, not when installing. It is really a problem? |
Module::Starter has a unique role in the templates it provides. They become accidental best practice and cargo cult. The contents usually end up sticking around not only through the whole distribution but also when it is handed off to other authors. We should take care in anything we introduce to them, that it is clear why everything is needed, and that it does not cause more problems than it helps. We would have much more freedom in, say, a Dist::Zilla minting profile, where its usage is specific. But here we are essentially handing out abacuses. I would prefer to keep it as footgun free as possible. |
Sadly a configure requires would not be appropriate here, even though the template already has the compatibility code for that. This would be a develop phase prereq, which Makefile.PL itself can't handle. |
Maybe we should change ExtUtils::MakeMaker to use ptar if available instead? |
Honestly, the last thing MakeMaker needs is new responsibilities. |
What do you mean by a new responsibilities? It is already ExtUtils::MakeMaker who generates Makefile. And if that generated Makefile is wrong, i.e. cause that So ExtUtils::MakeMaker is already responsible for creating distribution archive, so it should do it correctly. |
Figuring out "do we have a working ptar" or "do we have a working gnu tar", and in particular doing so portably. |
We can simply decide on the exact situation in which we're changing the default ( |
@xsawyerx That's because |
It's not clear to me why Devel::PPPort ended up with PAX headers, but adding them isn't the default on any system that I know of, aside from when the file metadata can't be stored in ustar format. |
@haarg When I run
(GNU tar) 1.30 |
Hi @haarg!
That is because new version of GNU /usr/bin/tar switched to PAX format by default.
Now it default on Fedora, OpenSUSE Leap and probably any rolling-edge linux distribution. And I guess this change would propagate to next SLES, RHEL or Debian stable versions too. |
I have released multiple distributions with tar 1.32 on Fedora 30 and it does not add the headers. Are you sure it is not just adding them because of something the ustar format can't store, like before? |
That is interesting. Maybe it is really because of something which ustar cannot store. In any case it is happening which means /usr/bin/tar is doing it and therefore Makefile should be fixed to not generate PAXed tarballs. |
If it is something ustar can't store, then forcing it to ustar means those will not get stored in the tarball at all, as @haarg said earlier in the ticket. |
Please, download tars.zip from my site and tell me what the tar.tar stores in the pax headers. It's missing in the other two, ptar.tar and ustar.tar. Is it something we need to store in a distribution? |
It's storing ctime, mtime, and atime in the pax headers. Pax headers allow for high resolution times than ustar, which may be why they are being used. The problematic Devel-PPPort release was only storing mtime, and only for the For this case, specifying --format=ustar should solve the issue, because tar will just store the less accurate time. |
This should prevent the module from being uninstallable on systems
that don't support PAX headers, but the tar of the creator of the
distribution supports them.
See e.g. Dual-Life/Devel-PPPort#183 or
https://huntingbears.nl/2015/02/17/new-tar-paxheaders-and-installing-from-cpan/