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

Add --enable-executable-static flag for fully static linking #5446

Merged
merged 2 commits into from
Feb 7, 2019

Conversation

nh2
Copy link
Member

@nh2 nh2 commented Jul 18, 2018

Fixes #391.

Also update the docs for --enable-executable-dynamic
as they were slightly misleading.

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!

  • I tested this by successfully building a big project (stack) with musl via nix using this new flag

My TODO list I kept when writing this code:

  • check if handling of withFullyStaticExe in Cabal/Distribution/Simple/GHCJS.hs is necessary (consider ocurrences of withDynExe)
  • exclusiveness checks:
    • how to ensure that withFullyStaticExe and withDynExe in LocalBuildInfo are mutually exclusive?
      • done
    • how to ensure that configFullyStaticExe and configDynExe in ConfigFlags are mutually exclusive?
    • add tests for the above
      • would appreciate feedback where those should go; can I even do that in Cabal's test suite if I need e.g. musl to test it?

Inputs appreciated for the above.

@nh2
Copy link
Member Author

nh2 commented Jul 18, 2018

Note this could also be done slightly differently, making the -optl=-static flag that we're passing to GHC here more generic.

As per man ld, the -static and -shared can be passed to the linker together, which

means that a shared library is being created but that all of the library's external references must be resolved by pulling in entries from static libraries".

That means it might be sensible in some use cases to pass -optl=-static when we are building a Haskell shared library (on Linux that is building an .so file that doesn't depend on any other .so files).

If we want to support that case, then maybe we don't want to call this feature --enable-executable-static, but something like --enable-fully-static-linking instead and have it work for building static exes and ".so file that doesn't depend on any other .so files".

But that those aren't necessarily exclusive; Cabal may want to offer a high-level flag called --enable-executable-static anyway and a separate one to allow the ".so file that doesn't depend on any other .so files" use case.

Just some ideas in my head, what do you think?

@@ -420,6 +420,11 @@ configure (pkg_descr0, pbi) cfg = do
die' verbosity $ "--enable-tests/--enable-benchmarks are incompatible with" ++
" explicitly specifying a component to configure."

-- Some sanity checks related to dynamic/static linking.
when (fromFlag (configDynExe cfg) && fromFlag (configFullyStaticExe cfg)) $
die' verbosity $ "--enable-executable-dynamic and --enable-executable-dynamic" ++
Copy link
Member Author

@nh2 nh2 Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Typo, one of them should be -static

@nh2 nh2 force-pushed the enable-static-executables-flag branch from da2d4a0 to 3ad4098 Compare July 18, 2018 23:23
@nh2 nh2 changed the title Add --enable-executable-static flag for fully static executables Add --enable-executable-static flag for fully static linking Jul 18, 2018
@angerman
Copy link
Collaborator

CI should agree though.

@nh2
Copy link
Member Author

nh2 commented Jul 20, 2018

  • I am unsure whether I should also pass -static in GHCJS.hs as I do in GHC.hs

Who can tell me?

@nh2 nh2 force-pushed the enable-static-executables-flag branch from e892df7 to 67ef73f Compare July 20, 2018 22:57
@nh2
Copy link
Member Author

nh2 commented Jul 20, 2018

I have added the corresponding cabal-install parts, please take another look.

@@ -654,7 +657,7 @@ instance Arbitrary PackageConfig where
, packageConfigHaddockContents = x40'
, packageConfigHaddockForHackage = x41' }
| (((x00', x01', x02', x03', x04'),
(x05', x42', x06', x07', x08', x09'),
(x05', x42', x06', x44', x07', x08', x09'),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What on earth was the author of the surrounding lines thinking?

The above arbitrary <*> arbitrary <*> arbitrary on 46 positional arguments is already contributor unfriendly but this one really sets a new bar.

@nh2 nh2 mentioned this pull request Jul 20, 2018
5 tasks
@nh2
Copy link
Member Author

nh2 commented Jul 21, 2018

Some gold tests seem to fail.

How can I run them locally?

The travis output on https://travis-ci.org/haskell/cabal/jobs/406458613 is close to useless.

The raw output at https://api.travis-ci.org/v3/job/406458613/log.txt has lots of garbled characters like �[0Ktravis_fold:end:before_cache.2 and seems to contain the actual test case failures like unrecognized 'configure' option --disable-executable-static'` somewhere in the middle.

@Ericson2314
Copy link
Collaborator

-static + -shared sounds like it would be good for foreign-libraries, which as root package / final artifact should be controlled with --enable-executable-static despite the weird naming.

GHCJS doesn't have alternate linking, so it really shouldn't matter.

@nh2
Copy link
Member Author

nh2 commented Oct 16, 2018

I think this is the 4th time it complains about ChangeLog.md being conflicting because somebody else has pushed a change meanwhile. I am getting tired of needlessly rebasing due to this.

I've filed #5625 for it.

@matthewbauer
Copy link
Collaborator

matthewbauer commented Dec 6, 2018

@nh2 I would recommend ignoring this by default on macOS. While its theoretically possible to build completely static macOS binaries, Apple doesn't provide anything like the stable ABI that Linux has. You can of course build all of your libraries statically but no matter what you will need to link against /usr/lib/libSystem.B.dylib. I am not sure what would happen when this flag is on in macOS? It might actually just ignore it.

@nh2
Copy link
Member Author

nh2 commented Dec 6, 2018

I would recommend ignoring this by default on macOS

@matthewbauer Would there be a benefit of Cabal ignoring the flag though, vs just having users not use it on OSX?

Accepting the flag and then going not doing anything goes against the principle of least surprise, I'd rather have it fail (and put into the docs that it isn't known to work on OSX), or fail loudly that it doesn't work on OSX (and why).

@nh2
Copy link
Member Author

nh2 commented Dec 6, 2018

I think this is the 4th time it complains about ChangeLog.md

and another time:

This branch has conflicts that must be resolved
Conflicting files
Cabal/ChangeLog.md

@23Skidoo I am unsure right now whether #5625 (comment) means that this PR will actually merge cleanly if not done via GitHub, or not.

@harpocrates
Copy link
Collaborator

I would recommend ignoring this by default on macOS

@matthewbauer Would there be a benefit of Cabal ignoring the flag though, vs just having users not use it on OSX?

Accepting the flag and then going not doing anything goes against the principle of least surprise, I'd rather have it fail (and put into the docs that it isn't known to work on OSX), or fail loudly that it doesn't work on OSX (and why).

I'd advocate for uniform behaviour: let the option work on all OSes, but add an explanatory warning on Mac. In other words: discourage over completely preventing.

@23Skidoo
Copy link
Member

23Skidoo commented Dec 6, 2018

@nh2 I tried rebasing, and it didn't require manual intervention, but put the changelog note into the 2.4.0.0 section, so it had to be fixed manually.

@23Skidoo 23Skidoo force-pushed the enable-static-executables-flag branch 2 times, most recently from e396797 to 1429bdb Compare December 6, 2018 08:49
@jkachmar
Copy link

Is there anything that an interested passerby like myself could do to help get this merged?

Statically linked Haskell executables are something I'm very much interested, so I'd like to help unblock anything if it's at all within my capability 😄

@jessekempf
Copy link

@23Skidoo: What's holding this up from being merged?

Also update the docs for `--enable-executable-dynamic`
as they were slightly misleading.
@23Skidoo 23Skidoo force-pushed the enable-static-executables-flag branch from 1429bdb to 2bff6ce Compare February 7, 2019 21:35
@23Skidoo
Copy link
Member

23Skidoo commented Feb 7, 2019

@nh2 wanted to add some tests for this. I think I'll merge this anyway rn, but a subsequent PR with tests would be appreciated!

@23Skidoo 23Skidoo merged commit a4c9925 into haskell:master Feb 7, 2019
@23Skidoo
Copy link
Member

23Skidoo commented Feb 7, 2019

Merged, thanks!

@nh2
Copy link
Member Author

nh2 commented Feb 7, 2019

Regarding tests, I'm still kinda blocked on

  • would appreciate feedback where those should go; can I even do that in Cabal's test suite if I need e.g. musl to test it?

How would you want something like this to happen?

Feature detection, and skipping these tests when musl isn't available?

@23Skidoo
Copy link
Member

23Skidoo commented Feb 8, 2019

Yeah, that'd be fine. Can we install musl on Travis?

@sboosali
Copy link
Collaborator

(I haven't done anything with Travis besides copy-paste-ing configuration files, but...) googling "travis musl":

travis-ci/travis-ci#6128

it sounds like there's no official support, but one can still just install a virtual machine with alpine linux (or some other environment with musl present).

i just set up Alpine Linux under Virtualbox on Ubuntu LTS 16.04 (afaik, that's the OS which Travis CI runs stuff on by default).

@nh2 do we need install to the cabal toolchain on it? or just check that any statically linked executables built by cabal ----enable-executable-static on the same platform (64-bit Intel, Linux) run (and/or pass some tests like ldd hello | grep "not a dynamic executable")?

@nh2
Copy link
Member Author

nh2 commented Feb 14, 2019

@sboosali Hmm, I think you will need a full musl toolchain all the way up to ghc for it to work.

I would be surprised if running a VM on Travis's Ubuntu works well (their environments are already quite slow, and if they don't have nested virtualisation, it might be unbearably slow).

Another alternative might be to link statically with glibc, and pick a very trivial hello-world program that is unlikely to trigger any code paths that are problematic when linking glibc statically.

hvr added a commit that referenced this pull request Mar 3, 2019
This was missed in c9f02b2 / #5446
and has been causing CI issues ever since.
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.

None yet

9 participants