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

Easier config to mirror Hackage #5870

Closed
brandonchinn178 opened this issue Sep 19, 2022 · 9 comments
Closed

Easier config to mirror Hackage #5870

brandonchinn178 opened this issue Sep 19, 2022 · 9 comments

Comments

@brandonchinn178
Copy link

Our company has an Artifactory instance that we've set up to mirror/cache Hackage urls; i.e. https://artifactory.example.com/hackage/foo/bar will mirror and cache https://hackage.haskell.org/foo/bar.

Currently it's annoying to configure Stack to use this mirror; currently we have to manually create ~/.stack/config.yaml containing

package-indices:
- download-prefix: https://artifactory.example.com/hackage/
  hackage-security:
    keyids:
    - ...
    key-threshold: 3
    ignore-expiry: true

Since the mirror is basically the same thing as Hackage, there's no point in duplicating all the keyids and whatnot; presumably this is all information already bundled in the Stack executable anyway.

It would be nice to have a way to just specify a URL and have it configured with the default configuration, e.g.:

# environment variable
export STACK_HACKAGE_URL=https://artifactory.example.com/hackage/

# helper command
stack config set hackage-url https://artifactory.example.com/hackage/
@mpilgrem
Copy link
Member

To make the hackage-security: limb optional, with its own default, seems to me sensible. Thinking about that:

Looking at the current package-indices: - structured as a list with only one item - it seems like an accident of history.

Also, I see the underlying pantry package provides a type Pantry.HackageSecurityConfig with a FromJSON instance that covers both the download-prefix limb and the hackage-security limb (and requires them all to be present).

Perhaps, ultimately, it would be better to have (the singular):

package-index:
  download-prefix:
  hackage-security:
    ...

and to separate out the hackage-security: limb into its own type with its own FromJSON instance, so that it could have its own default if not specified.

Then, some users could validly specify in their config.yaml (a configuration file that Stack creates, if it is missing) or stack.yaml:

package-index:
  download-prefix: https://artifactory.example.com/hackage/

I suppose the changes would need to start with changes to the Pantry API. Perhaps Pantry could have:

-- Replaces the existing HaskellSecurityConfig (which seems like a misnomer)
data PackageIndexConfig = PackageIndexConfig
  { picDownloadPrefix :: !Text
  , picHaskellSecurityConfig :: !HaskellSecurityConfig
  }

-- A new HaskellSecurityConfig, that deals with only the Haskell Security aspects
data HaskellSecurityConfig = HaskellSecurityConfig
  { hscKeyIds :: ![Text]
  , hscKeyThreshold :: !Int
  , hscIgnoreExpiry :: !Bool
  }

@hasufell
Copy link
Contributor

You should consider the possibility that you have multiple prefixes. Otherwise the design is too limited.

@mpilgrem
Copy link
Member

mpilgrem commented Oct 10, 2022

@hasufell, on the possibility of multiple prefixes, I can't immediately see how that possibility would arise. Currently Stack uses 'the' package index, either Hackage or (if so configured) a mirror of Hackage. If a Stack user is using 'Mirror A' of Hackage but wants to shift to 'Mirror B' of Hackage, they re-configure Stack. I am imaging a command stack config set package-index download-prefix URL that would make re-configuration easy at the command line.

Edit: the current Stack.Config.configFromConfigMonoid includes the following, which is what enforces the current structure of a list with only one item:

hsc <-
       case getFirst configMonoidPackageIndices of
         Nothing -> pure defaultHackageSecurityConfig
         Just [hsc] -> pure hsc
         Just x -> error $ "When overriding the default package index, you must provide exactly one value, received: " ++ show x

@mpilgrem
Copy link
Member

mpilgrem commented Oct 10, 2022

I am planning (subject to my proposals being acceptable to Stack collaborators) to implement this in four stages: (1) change the Pantry API - pantry-0.6.0; (2) change Stack to use the new Pantry API; (3) change Stack to implement the default desired by this request; (4) extend Stack to add the stack config set command.

(1) and (2) would happen at the same time (and would need an upper bound for the pantry dependency to be added to stack <= 2.9.1 on Hackage) - subject to my local testing, I think I have something almost ready to be proposed.

(3) and (4) are, I think, relatively straightforward once (1) and (2) are in place. EDIT: (4) may require a little thought because the current stack config set commands only deal with top level keys in the YAML configuration files.

mpilgrem added a commit to commercialhaskell/pantry that referenced this issue Oct 11, 2022
…fig`

This pull request proposes a relatively minor, but breaking, change to Pantry's API. There is a corresponding pull request for Stack that adopts this version of the Pantry API.

The motivation for this proposal is the request at Stack issue [#5870](commercialhaskell/stack#5870).

What is currently referred to as `HackageSecurityConfig` is better named `PackageIndexConfig`, comprising a true `HackageSecuityConfig` value and a download prefix (`picDownloadPrefix :: !Text`).

What is currently referred to as `defaultHackageSecurityConfig` is renamed `defaultPackageIndexConfig`.

True `HackageSecurityConfig` (strictly, `WithJSONWarnings HackageSecurityConfig`) has its own instance of `FromJSON` and its own default: `defaultHackageSecurityConfig`.

Meeting the ask in the Stack issue, the instance of `FromJSON` for `PackageIndexConfig` now assigns a default value of `defaultHackageSecurityConfig` if the `hackage-security` key is absent from the JSON object.

The field of `PackageConfig` named `pcHackageSecuity :: !HackageSecurityConfig` is renamed `pcPackageIndex :: !PackageIndexConfig`.

For completeness, `defaultDownloadPrefix` is also exposed.
mpilgrem added a commit that referenced this issue Oct 11, 2022
See also the related Pantry pull request: commercialhaskell/pantry#61.
@mpilgrem
Copy link
Member

I have raised pull requests for steps (1) (in Pantry) and (2) (in Stack) referred to in #5870 (comment) above.

@brandonchinn178, steps (1) and (2) alone should give effect to the default - because I decided to propose the default in the Pantry change rather than in Stack. I've tested by building a minimal program with a stack.yaml:

resolver: lts-19.28
packages:
- .

package-indices:
- download-prefix: https://artifactory.example.com/hackage/

mpilgrem added a commit to commercialhaskell/pantry that referenced this issue Oct 13, 2022
…fig`

This pull request proposes a relatively minor, but breaking, change to Pantry's API. There is a corresponding pull request for Stack that adopts this version of the Pantry API.

The motivation for this proposal is the request at Stack issue [#5870](commercialhaskell/stack#5870).

What is currently referred to as `HackageSecurityConfig` is better named `PackageIndexConfig`, comprising a true `HackageSecuityConfig` value and a download prefix (`picDownloadPrefix :: !Text`).

What is currently referred to as `defaultHackageSecurityConfig` is renamed `defaultPackageIndexConfig`.

True `HackageSecurityConfig` (strictly, `WithJSONWarnings HackageSecurityConfig`) has its own instance of `FromJSON` and its own default: `defaultHackageSecurityConfig`.

Meeting the ask in the Stack issue, the instance of `FromJSON` for `PackageIndexConfig` now assigns a default value of `defaultHackageSecurityConfig` if the `hackage-security` key is absent from the JSON object.

The field of `PackageConfig` named `pcHackageSecuity :: !HackageSecurityConfig` is renamed `pcPackageIndex :: !PackageIndexConfig`.

For completeness, `defaultDownloadPrefix` is also exposed.
mpilgrem added a commit that referenced this issue Oct 13, 2022
See also the related Pantry pull request: commercialhaskell/pantry#61.
mpilgrem added a commit that referenced this issue Oct 13, 2022
See also the related Pantry pull request: commercialhaskell/pantry#61.
mpilgrem added a commit that referenced this issue Oct 13, 2022
See also the related Pantry pull request: commercialhaskell/pantry#61.
mpilgrem added a commit that referenced this issue Oct 13, 2022
See also the related Pantry pull request: commercialhaskell/pantry#61.
mpilgrem added a commit that referenced this issue Oct 14, 2022
Fix #5870 Provide default for `hackage-security` key
@mpilgrem
Copy link
Member

pantry-0.6.0 is released on Hackage and the version of Stack in the master branch now makes use of it. I've not yet addressed the name of the key (package-index: v package-indices: []) or stack config set.

@mpilgrem mpilgrem reopened this Oct 14, 2022
mpilgrem added a commit that referenced this issue Oct 16, 2022
mpilgrem added a commit that referenced this issue Oct 19, 2022
@mpilgrem
Copy link
Member

Stack in the master branch now provides a package-index key.

@mpilgrem mpilgrem reopened this Oct 19, 2022
mpilgrem added a commit that referenced this issue Oct 21, 2022
Fix #5870 Add `stack config set package-index download-prefix` command
@mpilgrem
Copy link
Member

@brandonchinn178, all of what I proposed to do to clear this issue is now reflected in the master branch of Stack.

@brandonchinn178
Copy link
Author

Thanks! Looks great, excited to use it!

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

No branches or pull requests

3 participants