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

Allow ${pkgroot} prefix. #4892

Merged
merged 6 commits into from
Jan 21, 2018
Merged

Allow ${pkgroot} prefix. #4892

merged 6 commits into from
Jan 21, 2018

Conversation

angerman
Copy link
Collaborator

This change adds the ability to pass --prefix=\${pkgroot}. It is on purpose undocumented, as it should be only used with absolute care.

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!

Fixes #4872

@angerman
Copy link
Collaborator Author

@23Skidoo, @hvr this is not good, AppVeyor seems dead:

Build started
git clone -q https://github.com/haskell/cabal.git C:\projects\cabal
git fetch -q origin +refs/pull/4892/merge:
git checkout -qf FETCH_HEAD
Running Install scripts
choco install -y ghc --version 8.0.2
Chocolatey v0.10.7
Installing the following packages:
ghc
By installing you accept licenses for the packages.
Progress: Downloading cabal 2.0.0.0... 100%
Progress: Downloading ghc 8.0.2... 100%
cabal v2.0.0.0 [Approved]
cabal package files install completed. Performing other installation steps.
 ShimGen has successfully created a shim for cabal.exe
 The install of cabal was successful.
  Software install location not explicitly set, could be in package or 
  default install location if installer.
ghc v8.0.2 [Approved]
ghc package files install completed. Performing other installation steps.
Attempt to get headers for https://downloads.haskell.org/~ghc/8.0.2/ghc-8.0.2-x86_64-unknown-mingw32.tar.xz failed.
  The remote file either doesn't exist, is unauthorized, or is forbidden for url 'https://downloads.haskell.org/~ghc/8.0.2/ghc-8.0.2-x86_64-unknown-mingw32.tar.xz'. Exception calling "GetResponse" with "0" argument(s): "The operation has timed out"
Downloading ghc 64 bit
  from 'https://downloads.haskell.org/~ghc/8.0.2/ghc-8.0.2-x86_64-unknown-mingw32.tar.xz'
ERROR: The remote file either doesn't exist, is unauthorized, or is forbidden for url 'https://downloads.haskell.org/~ghc/8.0.2/ghc-8.0.2-x86_64-unknown-mingw32.tar.xz'. Exception calling "GetResponse" with "0" argument(s): "The operation has timed out" 
This package is likely not broken for licensed users - see https://chocolatey.org/docs/features-private-cdn.
The install of ghc was NOT successful.
Error while running 'C:\ProgramData\chocolatey\lib\ghc\tools\chocolateyInstall.ps1'.
 See log for details.
Chocolatey installed 1/2 packages. 1 packages failed.
 See the log for details (C:\ProgramData\chocolatey\logs\chocolatey.log).
Failures
 - ghc (exited 404) - Error while running 'C:\ProgramData\chocolatey\lib\ghc\tools\chocolateyInstall.ps1'.
 See log for details.
Command exited with code 404

@23Skidoo
Copy link
Member

I'm OK with adding an undocumented dangerous feature, but it'd be nice if @dcoutts could take a look.

@angerman
Copy link
Collaborator Author

I think I'll go and add --target-package-db flags to install and copy. That should give that prefix meaning. Just to be clear, I'll try to see how much relocatable issues I will be able to lift. But I'd rather try this piecemeal.

@angerman
Copy link
Collaborator Author

@23Skidoo how does this look now?

@angerman
Copy link
Collaborator Author

@23Skidoo, @hvr any chance I can get a review of this an a prelimiary one of #4874? As the general plan (as I understood @bgamari) now is to try to go ahead with snowleopard/hadrian#445 for 8.4, both are pre-requisits. And I'd like to get them finished in Cabal as soon as I can. As such any critique or "please do this and that" would be much appreciated!

Sorry to bother :(

@@ -1663,6 +1665,12 @@ installOptions showOrParseArgs =
"Do not install anything, only print what would be installed."
installDryRun (\v flags -> flags { installDryRun = v })
trueArg

, option "" ["target-package-db"]
Copy link
Member

Choose a reason for hiding this comment

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

Can we make this a hidden option?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can you give me a pointer where I can find a hidden option? I've tried looking for hiddenOption, and anything with hidden and option, but didn't turn anything up.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@23Skidoo so I fail to figure out how to make this a hidden option. I also tried to set the help-text to "" as per suggestion by @hvr, but that didn't drop it from being displayed either.



-- |The location prefix for the /copy/ command.
data CopyDest
= NoCopyDest
| CopyTo FilePath
deriving (Eq, Show)
| CopyToDb FilePath
Copy link
Member

Choose a reason for hiding this comment

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

Needs a Haddock comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

copyDest (\v flags -> case copyDest flags of
NoFlag -> flags { copyDest = v }
Flag NoCopyDest -> flags { copyDest = v }
_ -> error "Use either 'destdir' or 'target-package-db'.")
Copy link
Member

Choose a reason for hiding this comment

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

Use either 'destdir' or 'target-package-db'

, but not both? I think that this breaks --destdir=foo --destdir=bar, which used to work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, if --target-package-db is given, --destdir makes no sense. But allowing to specify --destdir twice (that would just take the latter one, I believe), if that used to be the behaviour should definitely stay that way.

@23Skidoo
Copy link
Member

@angerman Hidden flags are currently implemented in a somewhat ad hoc way, see https://github.com/haskell/cabal/blob/master/cabal-install/Distribution/Client/Setup.hs#L1617 for an example.

@ezyang ezyang closed this Dec 30, 2017
@ezyang ezyang reopened this Dec 30, 2017
@angerman
Copy link
Collaborator Author

Note to self: hidden flag is missing to for this to close.

@angerman angerman self-assigned this Jan 19, 2018
This change adds the ability to pass `--prefix=\${pkgroot}`. It is on purpose undocumented, as it should be only used with absolute care.
@angerman
Copy link
Collaborator Author

@23Skidoo is this good to go with the hidden flags? (assuming CI doesn't throw up).

@23Skidoo
Copy link
Member

@angerman Yep, this is good to go.

@angerman angerman merged commit 429fbc7 into haskell:master Jan 21, 2018
This was referenced Feb 5, 2018
@23Skidoo 23Skidoo added this to the 2.2 milestone Feb 5, 2018
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.

Allow $topdir and ${pkgroot} as --prefix values.
3 participants