-
Notifications
You must be signed in to change notification settings - Fork 696
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
cabal install Agda
fails in Setup.hs
with executable-dynamic: True
in config file (Linux)
#9784
Comments
The definition of succeeds here is important: does it succeed in building a dynamically linked executable, or does it succeed because the flag is simply ignored? I'm pretty certain it is the latter, since before #9697 command line options to install are simply ignored (where as options in config will be applied)... |
So, in light of #9697 (and my previous comment), if this is it then the ticket can be closed. However, why does Agda fail when compiled with |
Indeed, the flag is ignored. Agda isn't linked in dynamically, see: https://github.com/agda/agda/actions/runs/8213327362/job/22464590517?pr=7172 Two more insights:
|
cabal install Agda
fails with executable-dynamic: True
in config filecabal install Agda
fails with executable-dynamic: True
in config file (Linux, Windows)
cabal install Agda
fails with executable-dynamic: True
in config file (Linux, Windows)cabal install Agda
fails in Setup.hs
with executable-dynamic: True
in config file (Linux, Windows)
Hi @alt-romes,
It's because:
The usual ways to specify library path are:
Unfortunately I'm not seeing the Also notice, that |
@ulidtko Ah, I see. Ultimately, I don't think this is Cabal's fault, but rather Agda's use of a custom Setup, which will work in some situations but won't be robust to every one (like with executable-dynamic). I suspect that the reason why it does not work with |
Can Cabal provide perhaps a "post-atomic-store-install" hook, to enable use-cases like Agda's? |
I don't think the problem would be solved by that. I just meant to say that some assumptions made by the Agda custom Setup may not hold true in nix-style Cabal, and provide examples of things that changed -- I was not saying that that particular thing is the problem. I would have to investigate. |
@alt-romes for your convenience, I've minimized a test case that reproduces the issue. Find it here https://gist.github.com/ulidtko/fd65f7e5d486744d0f70dbe1934c6894 If anyone gives me a pointer to cabal tests perhaps, I can try to integrate it as a proper test case. |
Besides the install itself failing, leave that aside for now @alt-romes — notice from @andreasabel OP
— I do reproduce this, too (on cabal 3.10.2.1). See gist. I believe just this detail is way beyond expected, and is the actual subject of this bug report. |
I will take a look at the reproducer later, thanks. As for the comment regarding that bit of the OP: I've replied in this thread explaining that I've already fixed that discrepancy in a recent pull request: #9784 (comment) (That part is outdated) |
I took a look at this. The cause for the failure is that the installation strategy of v2 cabal is currently incompatible with running dynamically linked executables in the post install hook in not (yet) supporting relocatable builds (perhaps Agda shouldn't run the executable in a post-install step). I think we may be able to fix in Cabal by making the installation slightly more relocatable (an alternative is for Agda to work around this limitation manually, but it'd be pretty ugly :), it depends on whether the solution gets merged which may be a bit risky, but I think could be worth it -- then again, it may not be wise to do a risky change like this to support a workflow which is not decidely something we want cc @gbaz ) Here's what happens:
What can we do to solve this? Basically, set the library path of the package library something relative to the location of the binary.
But as I'm finished writing this I realize it may not be this simple. The problem is the same problem which makes the path to the temporary directory be that monstrous thing with duplicated directory hiearchy: Nothing in Cabal guarantees that the storedir is the same as the installdir..., which means that defining the path to the dynamic library relative to the binary may not be possible. This may be a more fundamental limitation than what I had originally thought. A possible alternative on Agda's side:
|
This is incorrect right away @alt-romes. The build happens under yet another temporary directory:
BTW this one looks fine to me, not weird in any way. It also fits into 1 line. |
Sorry, I was not precise and mixed building and installing. -- The write-side protocol is:
--
-- * Create a unique temp dir and write all store entry files into it.
--
-- * Take a lock named after the 'UnitId' in question.
--
-- * Once holding the lock, check again for the existence of the final store
-- entry directory. If the entry exists then the process lost the race and it
-- must abandon, unlock and re-use the existing store entry. If the entry
-- does not exist then the process won the race and it can proceed.
--
-- * Register the package into the package db. Note that the files are not in
-- their final location at this stage so registration file checks may need
-- to be disabled.
--
-- * Atomically rename the temp dir to the final store entry location.
--
-- * Release the previously-acquired lock. I'll update my comment. |
I've mixed up some things incorrectly and need to look at this again with a clearer head. This doesn't make a lot of sense. |
I understood this from your analysis in #9654 @alt-romes. But this doesn't follow:
— in the most general case, yes. But nothing prevents us from optimizing the common case, right? Can cabal detect, that none of custom In defensive programming philosophy, the only guarantees you have — are the ones you've explicitly checked. |
Oh dear... I've conflated a few too many things in this conversation (I blame tiredness)... Cabal explicitly extends the I misjudged and misunderstood a few things while going through this and was a bit hasty. Let me come back tomorrow to this with a clear head. |
@alt-romes sure. In advance, These things influence what the dynamic linker's resolution does on Linux:
You can use |
@ulidtko wrote:
E.g. there is an example how to make a test in one of @alt-romes PRs: https://github.com/haskell/cabal/pull/9762/files#diff-6db13a6ade63e9c657436ae3057120fa9b5fbc52fbc3b79e8d7574722f28f7b0 (But make sure you sync with him on this, maybe Rodrigo will add it in course of a fix of this issue.) |
I've minimised the reproducer a bit (for Linux): It suffices to have a cabal package with both a library and an executable, use We don't even need to get to the install step, simply running the built executable in I'm convinced this is a Cabal bug (which I can reproduce in master). I'm trying to figure out why this works on macOS. |
Why does this not fail on macOS? Cabal invokes GHC to link the executable, and, on macOS, GHC will add RPATH entries to the executable pointing to libraries used by the executable (see Why does this not fail if build-type: Simple? When using
However, when using
Where to look now? This narrows it down: I suspect |
It seems that it is expected for an executable which depends on an internal lib built with Custom setup/legacy fallback and + -- Test that we don't accidentally add the inplace directory to
+ -- an executable RPATH. Don't test on Windows, which doesn't
+ -- support RPATH.
+ unlessWindows $ do
+ tc "Regression/T4025" $ do
+ cabal "configure" ["--enable-executable-dynamic"]
+ cabal "build" []
+ -- This should fail as it we should NOT be able to find the
+ -- dynamic library for the internal library (since we didn't
+ -- install it). If we incorrectly encoded our local dist
+ -- dir in the RPATH, this will succeed.
+ shouldFail $ runExe' "exe" [] On one hand I understand this point, but on the other hand it makes it impossible for packages with Custom setups to run the built executables in the copy phase, as the dynamic library will only be available where it is expected to be after it is installed in the store. |
I don't entirely follow #4033 on the first read. 😕 @alt-romes speaking of phases, what's the sequence? build, copy, register, install ? For context, what Agda does: it's a compiler that ships with a bunch of wired-in internal modules, a-la GHC.Prim. For importing, these modules must first be typechecked, producing Another reason is efficiency. Also, keeping permission boundaries: when run later by user, the compiler might no longer have permissions to write the That's why that custom setup hook is there. I don't think it's particularly important for Agda in which phase of cabal install it runs. The only expected outcomes are:
So maybe, it shouldn't be a hook in the copy phase, but some other phase? |
I've briefly discussed this with Duncan, and the summary of our discussion is:
|
cabal install Agda
fails in Setup.hs
with executable-dynamic: True
in config file (Linux, Windows)cabal install Agda
fails in Setup.hs
with executable-dynamic: True
in config file (Linux)
cabal install Agda
fails withexecutable-dynamic: True
in config file:What is puzzling that
cabal install Agda --enable-executable-dynamic
(instead of setting in the config file) succeeds: https://github.com/agda/agda/actions/runs/8184356501/job/22378664111?pr=7172Agda fails in the
copyHook
in itsSetup.hs
when it tries to invoke the just-built executable: https://github.com/agda/agda/blob/dee0d2e1300b43ac91517c3a5c58cae6b6af3a3d/Setup.hs#L134-L140This situation has been discussed elsewhere in this issue tracker:
The current issue highlights the discrepancy in behavior between passing the command-line flag and changing the config setting for dynamic linking.
Related:
ghc-options: -dynamic
: v2-build does not pass ghc-options in.cabal/config
to ghc when building custom Setup.hs #6505The text was updated successfully, but these errors were encountered: