haskell: move generateOptparseApplicativeCompletions into pkg sets for cross support #190536
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sternenseemann
requested review from
cdepillabout and
maralorn
as code owners
github-actions
bot
added
6.topic: haskell
6.topic: nixos
ofborg
bot
added
10.rebuild-darwin: 0
cdepillabout
reviewed
Sep 9, 2022
Comment on lines
420
to
422
| addOptparseApplicativeCompletionScripts = exeName: pkg: | ||
| builtins.trace "addOptparseApplicativeCompletionScripts is deprecated in favor of generateOptparseApplicativeCompletion. Please change ${pkg.name} to use the latter or its plural form." | ||
| (generateOptparseApplicativeCompletion exeName pkg); | ||
| lib.warn "addOptparseApplicativeCompletionScripts is deprecated in favor of haskellPackages.generateOptparseApplicativeCompletions. Please change ${pkg.name} to use the latter and make sure it uses its matching haskell.packages set!" | ||
| (__generateOptparseApplicativeCompletion exeName pkg); |
There was a problem hiding this comment.
I don't feel strongly about this, but maybe we could just remove this function? It has been here and been deprecated for quite a long time, right?
I can't ever recall seeing this in the wild.
There was a problem hiding this comment.
Deleting it form haskell.lib.compose should be fine in any case, we can still keep it in haskell.lib for backwards compatibility.
cdepillabout
approved these changes
Sep 9, 2022
Artturin
reviewed
Sep 9, 2022
sternenseemann
force-pushed
the
generate-opt-parse-completion-cross
branch
from
3aa3e54
to
31e1c09
Compare
31 tasks
roberth
requested changes
Oct 4, 2022
| */ | ||
| generateOptparseApplicativeCompletions = commands: pkg: | ||
| pkgs.lib.foldr generateOptparseApplicativeCompletion pkg commands; | ||
| lib.warn "haskellLib.generateOptparseApplicativeCompletions is deprecated in favor of haskellPackages.generateOptparseApplicativeCompletions. Please change ${pkg.name} to use the latter and make sure it uses its matching haskell.packages set!" |
There was a problem hiding this comment.
Suggested change
| lib.warn "haskellLib.generateOptparseApplicativeCompletions is deprecated in favor of haskellPackages.generateOptparseApplicativeCompletions. Please change ${pkg.name} to use the latter and make sure it uses its matching haskell.packages set!" | |
| lib.warnIf (lib.isInOldestRelease 2211) "haskellLib.generateOptparseApplicativeCompletions is deprecated in favor of haskellPackages.generateOptparseApplicativeCompletions. Please change ${pkg.name} to use the latter and make sure it uses its matching haskell.packages set!" |
We should give third-party projects a bit of time before warning, so that they aren't asked to apply the change before all supported Nixpkgs releases support the new method.
There was a problem hiding this comment.
Side note: The naming of this function is so confusing, it took me like 5 minutes to figure out what it means.
There was a problem hiding this comment.
Not sure which one you mean, but I wrote both and I agree. I'm open to suggestions.
ofborg
bot
added
the
2.status: merge conflict
Deprecate haskell.lib{,.compose}.generateOptparseApplicativeCompletion*
in favor of the newly added
haskell.packages.*.generateOptparseApplicativeCompletions (plural!)
which takes into account whether we are cross-compiling or not. If we
are, generating completions is disabled, since we can't execute software
built for a different platform.
The move is necessary, so we can receive the /same/ stdenv as the
package we are overriding in order to accurately check whether we can
execute produced binaries.
Resolves NixOS#174040.
Resolves NixOS#49648.
This function was deprecated when it was first added to haskell.lib.compose, so we can probably safely drop it.
sternenseemann
force-pushed
the
generate-opt-parse-completion-cross
branch
from
5a8fa99
to
68df602
Compare
ofborg
bot
removed
the
2.status: merge conflict
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Deprecate haskell.lib{,.compose}.generateOptparseApplicativeCompletion* in favor of the newly added
haskell.packages.*.generateOptparseApplicativeCompletions (plural!) which takes into account whether we are cross-compiling or not. If we are, generating completions is disabled, since we can't execute software built for a different platform.
The move is necessary, so we can receive the /same/ stdenv as the package we are overriding in order to accurately check whether we can execute produced binaries.
Resolves #174040.
Resolves #49748.
Description of changes
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes