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

Support for non-standard program search $PATH #1415

Merged
merged 8 commits into from Aug 19, 2013

Conversation

Projects
None yet
3 participants
@dcoutts
Copy link
Member

dcoutts commented Aug 10, 2013

Various internal infrastructure changes, but ultimately it should help us solve the problem where new users install programs like alex/happy/c2hs etc and then cabal cannot find the programs they just installed, because it puts them in a place that is not on the $PATH.

New users will get an entry in their ~/.cabal/config like:

extra-prog-path: /home/duncan/.cabal/bin

And this dir will also be searched for programs, along with the $PATH.

Internally we add some infrastructure for allowing configured programs to have arbitrary env var overrides. We could also use this for other purposes (e.g. ignoring the GHC_PACKAGE_PATH env var by unsetting it in all calls to ghc.)

It also means that internally in the library we can have a local "virtual" $PATH, rather than having to share the process-global one.

dcoutts added some commits Aug 10, 2013

Flesh out the Program run code to cover all cases
Previously the runProgramInvocation impl only covered the common special
cases that we currently use. In particular it did not support env vars
or changing the workind directory.

Also change the ProgramInvocation { progInvokeEnv } to allow unsetting
env vars, not just setting them to new values.

This stuff would be better if we could use createProcess for all cases
but that doesn't yet support running programs in the foreground with
proper ctl-C handling (only rawSystem currently supports that).
Add a programOverrideEnv to the ConfiguredProgram
And use it in programInvocation. So configured programs can have
environment var overrides that will be used in all invocations, like
the existing support for default and override arguments.
Allow the Program programPostConf to update all settings, including env
So rather than only returning the default args it can adjust any of the
ConfiguredProgram settings, including programDefaultArgs as now but now
also the programOverrideArgs and the new programOverrideEnv.
Add a notion of program search path to the ProgramDb
Previously we would just use the normal system $PATH for finding
programs (unless the Program provided some custom method). Now the
ProgramDb has its own notion of the search path and we use that for
finding programs (by default). The search path can be either specific
directories or the system search method (ie $PATH on unix and something
similar on Win32). The default search path is just the system one.

In addition, this search path is passed on to programs when we invoke
them as the $PATH env var.
Take the opportunity to simplify the ghc tool configuration
When we configure ghc we have to find a bunch of related tools (like gcc
on windows). We do that by looking in some extra locations first, and
that's now simpler to do with the new infrastructure by just tacking an
extra directory on the front of the search path.
Add a configure --extra-prog-path flag
Can be used to add extra dirs to the end of the program search path.
This in mainly useful as a config file entry rather than a command line
flag, but it'll exists as both.
Use the --extra-prog-path flag in cabal-install
We have to pass it properly in the setupWrapper. For the external build
method we just set the $PATH rather than passing the flag. This way it
works when talking to Setup.hs built with an older Cabal lib.
Add a sensible extra-prog-path to the default ~/.cabal/config
e.g. extra-prog-path: /home/duncan/.cabal/bin

This should help with the problem that people install tools like alex,
happy, c2hs etc and then cabal cannot find them.

Arguably we should have this as the default value for extra-prog-path,
so that it affects existing installations, rather than just affecting
new installations where we generate the ~/.cabal/config for the first
time. It's easy to change if we want to do that.
@tibbe

This comment has been minimized.

Copy link
Member

tibbe commented Aug 10, 2013

What's the precedence of extra-prog-path in relation to the $PATH?

@dcoutts

This comment has been minimized.

Copy link
Member

dcoutts commented Aug 12, 2013

@tibbe It comes after. It it's PATH=$PATH:$extra-prog-path. I thought this was consistent with the other -extra-blah flags. We can of course have an override one, or change the UI completely.

@tibbe

This comment has been minimized.

Copy link
Member

tibbe commented Aug 12, 2013

The current behavior sounds reasonable to me.

@23Skidoo

This comment has been minimized.

Copy link

23Skidoo commented on 50af0d7 Aug 16, 2013

LGTM.

@23Skidoo

This comment has been minimized.

Copy link

23Skidoo commented on b0f08f4 Aug 16, 2013

LGTM.

@23Skidoo

This comment has been minimized.

Copy link

23Skidoo commented on 34a2c8c Aug 16, 2013

LGTM.

@23Skidoo

This comment has been minimized.

Typo: s/analagous/analogous/.

@23Skidoo

This comment has been minimized.

Possible improvement: read the list of extensions from the PATHEXT env var. By default PATHEXT is ".com; .exe; .bat; .cmd".

@23Skidoo

This comment has been minimized.

Perhaps we can optimise the default case ([ProgramSearchPathDefault]) and just inherit the value of PATH from the parent process?

This comment has been minimized.

Copy link

23Skidoo replied Aug 16, 2013

...although since we now add ~/.cabal/bin to the search path by default, this probably doesn't matter.

@23Skidoo

This comment has been minimized.

Copy link

23Skidoo commented on 1648fa4 Aug 16, 2013

LGTM.

@23Skidoo

This comment has been minimized.

Perhaps it'd be nice if the duplication here was factored out.

@23Skidoo

This comment has been minimized.

Copy link

23Skidoo commented on 046c086 Aug 16, 2013

LGTM.

@23Skidoo

This comment has been minimized.

Copy link

23Skidoo commented on 66a1bbf Aug 16, 2013

LGTM.

@23Skidoo

This comment has been minimized.

Hmm, you pass in options', but use options here. Is this right?

@23Skidoo

This comment has been minimized.

Copy link
Member

23Skidoo commented Aug 16, 2013

Looks good in general, see some minor comments.

23Skidoo added a commit that referenced this pull request Aug 19, 2013

Merge pull request #1415 from dcoutts/env-searchpath
Support for non-standard program search $PATH

@23Skidoo 23Skidoo merged commit e778445 into haskell:master Aug 19, 2013

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