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

[cabal-7825] Add $CABAL env variable for external commands #9393

Closed
wants to merge 1 commit into from

Conversation

yvan-sraka
Copy link
Collaborator

@yvan-sraka yvan-sraka commented Nov 3, 2023

This is a follow-up of #9063

Checklist:

QA notes:

I tested it as followed, on my machine I have 2 scripts:

  • cabal-foobar:

    #! /usr/bin/env bash
    env | grep CABAL
  • test-ext-cmd.sh

    #! /usr/bin/env bash
    PATH="$PATH:." cabal run cabal-install foobar

Then, e.g., as expected:

% ./test-ext-cmd.sh
CABAL=/Users/yvan/IOHK/cabal/dist-newstyle/build/aarch64-osx/ghc-9.2.8/cabal-install-3.11.0.0/x/cabal/build/cabal/cabal

@yvan-sraka yvan-sraka requested a review from mpickering November 3, 2023 11:13
@yvan-sraka yvan-sraka self-assigned this Nov 3, 2023
@@ -663,7 +664,8 @@ commandsRun globalCommand commands args =

callExternal :: a -> String -> [String] -> IO (CommandParse (a, CommandParse action))
callExternal flags exec cmdArgs = do
result <- try $ callProcess exec cmdArgs
execPath <- getExecutablePath
result <- try $ createProcess (proc exec cmdArgs){env = Just [("CABAL", execPath)]}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Closer.. but setting env like this overrides the whole environment, you need to get the current environment first using getEnvironment.

@mpickering
Copy link
Collaborator

I have written some simple tests for the external command interface (which would fail with this MR). Perhaps I should commit those first and then we can rebase this MR on top?

@yvan-sraka
Copy link
Collaborator Author

@mpickering AFAIU this PR could be closed since in #9412 you:

  • Set the $CABAL environment variable to the current executable path
    • This allows external commands to be implemented by calling $CABAL,
      which is strongly preferred to linking against the Cabal library as
      there is no easy way to guantee your tool and cabal-install link
      against the same Cabal library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants