new-sdist adds executable permission to *.txt files

[peti]

The tarball generated by cabal new-sdist comes with several *.txt files having a +x file permission set even though these files don't have that bit set in the source directory they were pulled from. See the permissions of README.txt for example:

/tmp $ gcl git://github.com/Deewiant/glob.git
Cloning into 'glob'...
remote: Enumerating objects: 54, done.
remote: Counting objects: 100% (54/54), done.
remote: Compressing objects: 100% (35/35), done.
remote: Total 1939 (delta 19), reused 41 (delta 13), pack-reused 1885
Receiving objects: 100% (1939/1939), 248.79 KiB | 683.00 KiB/s, done.
Resolving deltas: 100% (1037/1037), done.

/tmp $ cd glob

/tmp/glob $ cabal new-sdist
Wrote tarball sdist to /tmp/glob/dist-newstyle/sdist/Glob-0.10.0.tar.gz
/tmp/glob $ tar tfv dist-newstyle/sdist/Glob-0.10.0.tar.gz
drwxr-xr-x 0/0               0 2001-09-09 03:46 Glob-0.10.0/
-rwxr-xr-x 0/0            9294 2001-09-09 03:46 Glob-0.10.0/CHANGELOG.txt
-rwxr-xr-x 0/0             112 2001-09-09 03:46 Glob-0.10.0/CREDITS.txt
-rw-r--r-- 0/0            2933 2001-09-09 03:46 Glob-0.10.0/Glob.cabal
-rw-r--r-- 0/0            1651 2001-09-09 03:46 Glob-0.10.0/LICENSE.txt
-rwxr-xr-x 0/0             746 2001-09-09 03:46 Glob-0.10.0/README.txt
-rw-r--r-- 0/0              46 2001-09-09 03:46 Glob-0.10.0/Setup.hs
drwxr-xr-x 0/0               0 2001-09-09 03:46 Glob-0.10.0/System/FilePath/
-rw-r--r-- 0/0            2378 2001-09-09 03:46 Glob-0.10.0/System/FilePath/Glob.hs
drwxr-xr-x 0/0               0 2001-09-09 03:46 Glob-0.10.0/System/FilePath/Glob/
-rw-r--r-- 0/0           25538 2001-09-09 03:46 Glob-0.10.0/System/FilePath/Glob/Base.hs
-rw-r--r-- 0/0           16160 2001-09-09 03:46 Glob-0.10.0/System/FilePath/Glob/Directory.hs
-rw-r--r-- 0/0            7260 2001-09-09 03:46 Glob-0.10.0/System/FilePath/Glob/Match.hs
-rw-r--r-- 0/0            2183 2001-09-09 03:46 Glob-0.10.0/System/FilePath/Glob/Primitive.hs
-rw-r--r-- 0/0            1477 2001-09-09 03:46 Glob-0.10.0/System/FilePath/Glob/Simplify.hs
-rw-r--r-- 0/0            4911 2001-09-09 03:46 Glob-0.10.0/System/FilePath/Glob/Utils.hs
drwxr-xr-x 0/0               0 2001-09-09 03:46 Glob-0.10.0/tests/
-rw-r--r-- 0/0             890 2001-09-09 03:46 Glob-0.10.0/tests/Main.hs
drwxr-xr-x 0/0               0 2001-09-09 03:46 Glob-0.10.0/tests/Tests/
-rw-r--r-- 0/0            2462 2001-09-09 03:46 Glob-0.10.0/tests/Tests/Base.hs
-rw-r--r-- 0/0            1221 2001-09-09 03:46 Glob-0.10.0/tests/Tests/Compiler.hs
-rw-r--r-- 0/0            5553 2001-09-09 03:46 Glob-0.10.0/tests/Tests/Directory.hs
-rw-r--r-- 0/0            2290 2001-09-09 03:46 Glob-0.10.0/tests/Tests/Instances.hs
-rw-r--r-- 0/0            2702 2001-09-09 03:46 Glob-0.10.0/tests/Tests/Matcher.hs
-rw-r--r-- 0/0            1724 2001-09-09 03:46 Glob-0.10.0/tests/Tests/Optimizer.hs
-rw-r--r-- 0/0            5534 2001-09-09 03:46 Glob-0.10.0/tests/Tests/Regression.hs
-rw-r--r-- 0/0            1034 2001-09-09 03:46 Glob-0.10.0/tests/Tests/Simplifier.hs
-rw-r--r-- 0/0            1632 2001-09-09 03:46 Glob-0.10.0/tests/Tests/Utils.hs

/tmp/glob $ ls -l *.txt
-rw-r--r-- 1 simons users 9294 Dec 22 21:36 CHANGELOG.txt
-rw-r--r-- 1 simons users  112 Dec 22 21:36 CREDITS.txt
-rw-r--r-- 1 simons users 1651 Dec 22 21:36 LICENSE.txt
-rw-r--r-- 1 simons users  746 Dec 22 21:36 README.txt

/tmp/glob $ cabal --version
cabal-install version 2.4.1.0
compiled using version 2.4.1.0 of the Cabal library

[typedrat]

That's just down to how it works internally, and I believe that v1-sdist does the same.

[peti]

Making README.txt an executable file doesn't seem like a good idea, though. Does it?

[typedrat]

It only matters if the user actually runs it... I suggest not running random .txt files in temp directories because you see they have the executable permission.

[peti]

It only matters if the user actually runs it...

Many Linux distributions have sophisticated QA mechanisms that prevent packages from doing stuff like installing executable files into /usr/share, etc. This weird feature breaks all those Haskell builds in openSUSE: https://build.opensuse.org/package/live_build_log/devel:languages:haskell:ghc-8.6.x/ghc-Glob/openSUSE_Tumbleweed/x86_64. That can't be a good thing, no?

[typedrat]

The patch is trivial, anyway. Instead of setting every potentially executable file as executable, check the current execute permissions and continue those forward.

[RyanGlScott]

Humorously enough, using cabal get to obtain a Hackage tarball "works around" this issue. This is because using the tar library's unpack function to unpack a tarball (as cabal-install does) causes executable permissions to be stripped away as a result of haskell/tar#25.

[asr]

That's just down to how it works internally, and I believe that v1-sdist does the same.

No, v1-sdist (on cabal-install 2.4.1.0) doesn't add executable permission to *.txt files.

[RyanGlScott]

Nor does v1-sdist add executable permissions on cabal-install-3.0.1.0. v1-sdist and v2-sdist appear to have very different code paths. Here is how v1-sdist writes a tarball:

cabal/Cabal/Distribution/Simple/SrcDist.hs

Lines 430 to 438 in 787b1f2

	runProgram verbosity tarProg $
	-- Hmm: I could well be skating on thinner ice here by using the -C option
	-- (=> seems to be supported at least by GNU and *BSD tar) [The
	-- prev. solution used pipes and sub-command sequences to set up the paths
	-- correctly, which is problematic in a Windows setting.]
	["-czf", tarBallFilePath, "-C", tmpDir]
	++ (if formatOptSupported then ["--format", "ustar"] else [])
	++ [tarBallName pkg_descr]
	return tarBallFilePath

And here is how v2-sdist writes a tarball:

cabal/cabal-install/Distribution/Client/CmdSdist.hs

Line 287 in f580d39

writeLBS . normalize . GZip.compress . Tar.write $ fmap setModTime entries

So v1-dist shells out to the tar program while v2-sdist uses the Haskell tar library. Perhaps the use of the tar library is why this code has to go through so much trouble to try and manually set file permissions in the first place? (Especially in light of haskell/tar#25.)

[asr]

cabal-install-3.0.1.0? Did you mean cabal-install-3.0.0.0?

[typedrat]

@RyanGlScott it's not optional, sadly. Relying on whatever tar program is to hand does not give us the level of control we require for v2-sdist. Like I said in a comment last year(!), there's an easy fix, I can try and get a PR in this weekend.

[RyanGlScott]

@asr:

cabal-install-3.0.1.0? Did you mean cabal-install-3.0.0.0?

No, I meant cabal-install-3.0.1.0:

$ /opt/cabal/3.0/bin/cabal --version
cabal-install version 3.0.1.0
compiled using version 3.0.1.0 of the Cabal library

I'm using cabal-install-3.0 obtained from https://launchpad.net/~hvr/+archive/ubuntu/ghc.

@typedrat:

there's an easy fix, I can try and get a PR in this weekend.

I'm not sure I understand how the fix works. From , you say:

Instead of setting every potentially executable file as executable, check the current execute permissions and continue those forward.

However, looking at the implementation:

cabal/cabal-install/Distribution/Client/CmdSdist.hs

Lines 259 to 261 in f580d39

	perm' = case perm of
	Exec -> Tar.executableFilePermissions
	NoExec -> Tar.ordinaryFilePermissions

It looks like v2-sdist always sets each file to have either executableFilePermissions (i.e., rwxr-xr-x) or ordinaryFilePermissions (i.e., rw-r--r--). What happens to files that don't use either of these two predefined sets of permissions?

[phadej]

Note, Cabal doesn't use tar library because it cannot depend on it due build system reasons.

Does we have tests that Cabal sdist and cabal-install sdist produce the same tarballs (module something irrelevant)?

Ultimately, this should be also checked on upload on Hackage, so no tarballs with executable bits are uploaded at all, as whoever can package .tar.gz and upload it. (E.g. Cabal is manually assembled, I have no idea what stack does etc.)

I.e. omitting the file permissions on unpack sounds very right thing to do.

In that light, as I commented on the PR, I don't see a reason from not setting any permissions at all, just 644 for files and 655 for directories unconditionally. Bundling native binaries is just wrong, and scripts can be run by invoking their interpreter (like is done with configure).

[juhp]

I tend to disagree: I don't see a reason to strip intended executable permissions from scripts: though I certainly prefer everything 644 to everything 755...

I think distros don't use cabal-install but tar to unpack.

[juhp]

Thank you

So I was trying to reproduce this to test the fix (but I need to work slightly harder to backport it to 2.4 for Fedora), and so far I found that the original problem occurs for Extra-Source-Files (maybe they were assumed to be scripts?) but not Extra-Doc-Files.

[juhp]

I finally tested this and can confirm that the issue is fixed, thank you!

[juhp]

Also if it helps others I backported the fix to cabal-install-2.4.1.0 in Fedora:
https://src.fedoraproject.org/rpms/cabal-install/blob/master/f/cabal-install-sdist-file-permissions.patch

[juhp]

(It is a little sad that this didn't even get into cabal-install-3.2.)