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 sdist permissions #6666

Merged
merged 4 commits into from
Apr 7, 2020
Merged

Fix sdist permissions #6666

merged 4 commits into from
Apr 7, 2020

Conversation

phadej
Copy link
Collaborator

@phadej phadej commented Apr 6, 2020


Please include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Any changes that could be relevant to users have been recorded in the changelog.
  • The documentation has been updated, if necessary.
  • If the change is docs-only, [ci skip] is used to avoid triggering the build bots.

Please also shortly describe how you tested your change. Bonus points for added tests!

@phadej phadej force-pushed the fix-sdist-permissions branch 2 times, most recently from 253db6b to e77222e Compare April 6, 2020 22:47
@phadej phadej force-pushed the fix-sdist-permissions branch from e77222e to f26175e Compare April 7, 2020 10:35
Also refactor ./Setup sdist to not do anything fancy.
Now `./Setup sdist` and `v2-sdist` should produce close(r)
to same tarballs.
@ramsdell
Copy link
Contributor

I have just been informed that Cabal sdist will make all files in Extra-Source-Files read-only. This is very bad for the CPSA distribution because Extra-Source-Files contains executable scripts. How do you propose executable scripts be included in a distribution?

@phadej
Copy link
Collaborator Author

phadej commented May 21, 2020

What kind of executable scripts. If they are truly scripts, execute them using explicitly calling an interpreter.

@ramsdell
Copy link
Contributor

We deliver several scripts. One script runs our tests suite. One script finds the largest amount of memory that can be safely used on a Linux system by consulting /proc/meminfo. It exports its result by setting GHCRTS. Another uses Python to perform a complex transformation on CPSA input. They are programs to be executed.

Why should one not be able to include executable scripts in a distribution? Seems arbitrary to fail to allow this.

@phadej
Copy link
Collaborator Author

phadej commented May 22, 2020

Because it's very hard for Cabal to know which files needs to be executable and which shouldn't. As your original issue mentioned, just marking all extra-source-files as executables is wrong as well.

I don't want rely on the existing file permissions, as those are very implicit. E.g. on Windows.

Additionally the packages may be unpacked on filesystems with noexec so just trying to execute scripts may fail even on windows. Discovering python and running the interpreter explicitly is a bit more work, but is not impossible, and overall better.

See e.g. how Cabal itself calls configure script: https://github.com/haskell/cabal/blob/master/Cabal/Distribution/Simple.hs#L692-L758

@ramsdell
Copy link
Contributor

Hmm. I thought Cabal was inferring permissions correctly in pre 3.0 versions of Cabal, but then I had strange problems with sdist on Macs, so I was always building distributions on Linux. (There was some weird issue with the size of uuid because my company uses employee numbers.) In any event, that is the past. Let me ask for something different to solve the problem.

Please add a new field called scripts which is behaves as does extra-source-files, with the only exception that it adds execute permissions to the files within the archive. Surely this must be easy to do.

@gbaz
Copy link
Collaborator

gbaz commented May 22, 2020

Can you please confirm why oleg's proposed solution (calling the script file explicitly with an interpreter) does not work?

@ramsdell
Copy link
Contributor

The user has to know what interpreter to call. A user should not need to know that. That is what the #! notation at the beginning of an executable script file is for.

@ramsdell
Copy link
Contributor

It just occurred to me that some people many not know about shebang in Unix. It's a standard feature that allows a text file to used as if it were an executable. The feature is enabled when the file is given execute permission and the first two characters of the file are #!. The characters that follow the shebang must specify the absolute path of an executable. Some typical examples are:

  • #!/bin/sh - execute the file using a Bourne shell or something compatible.
  • #!/usr/bin/env python - execute the file using the version of Python found by env.
  • #!/bin/false - ensure the file fails to execute even if it is given execute permissions.

As it says in Shebang, the purpose is to "allow scripts and data files to be used as commands, hiding the details of their implementation from users and other programs, by removing the need to prefix scripts with their interpreter on the command line."

A CPSA distribution contains several Bourne shell scripts and a few Python scripts.

@phadej phadej added this to the 3.4.0.0-rc1 milestone Jul 10, 2020
@phadej phadej mentioned this pull request Jul 10, 2020
frasertweedale added a commit to willdonnelly/dyre that referenced this pull request Mar 15, 2021
Cabal 3.4 strips the execute permission from extra source files;
see haskell/cabal#6666.  This impacts our
test scripts and causes CI to fail.

Execute test scripts via sh(1) to avoid test failure due to no
execute permission.
frasertweedale added a commit to willdonnelly/dyre that referenced this pull request Mar 15, 2021
Cabal 3.4 strips the execute permission from extra source files;
see haskell/cabal#6666.  This impacts our
test scripts and causes CI to fail.

Execute test scripts via sh(1) to avoid test failure due to no
execute permission.
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.

4 participants