Update nixpkgs and various associated fixes#389
Open
jfly wants to merge 6 commits into
Open
Conversation
Collaborator
Author
|
This PR is ready for review. It depends on #390, which proves that I addressed all the warnings introduced by the ghc update. |
jfly
changed the title
jfly
force-pushed
the
upgrade-nixpkgs
branch
2 times, most recently
from
789966d to
480e91d
Compare
We're upgrading to GHC 9.10.3, and it will start to emit warnings about
risky use of partial functions.
AFAICT, the old code wasn't wrong (we were always checking for empty
list before trying to call `head` on it, for example), but it's nice
when a compiler can check things for us!
```
$ cabal build
...
src/Nixfmt/Types.hs:375:68: warning: [GHC-63394] [-Wx-partial]
In the use of ‘head’
(imported from Prelude, but defined in GHC.Internal.List):
"This is a partial function, it throws an error on empty lists. Use pattern matching, 'Data.List.uncons' or 'Data.Maybe.listToMaybe' instead. Consider refactoring to use "Data.List.NonEmpty"."
|
375 | (List _ items _) | Prelude.length (unItems items) == 1 -> case Prelude.head (unItems items) of
| ^^^^^^^^^^^^
src/Nixfmt/Types.hs:384:69: warning: [GHC-63394] [-Wx-partial]
In the use of ‘head’
(imported from Prelude, but defined in GHC.Internal.List):
"This is a partial function, it throws an error on empty lists. Use pattern matching, 'Data.List.uncons' or 'Data.Maybe.listToMaybe' instead. Consider refactoring to use "Data.List.NonEmpty"."
|
384 | (Set _ _ items _) | Prelude.length (unItems items) == 1 -> case Prelude.head (unItems items) of
| ^^^^^^^^^^^^
[3 of 8] Compiling Nixfmt.Predoc ( src/Nixfmt/Predoc.hs, dist-newstyle/build/x86_64-linux/ghc-9.10.3/nixfmt-1.2.0/build/Nixfmt/Predoc.o, dist-newstyle/build/x86_64-linux/ghc-9.10.3/nixfmt-1.2.0/build/Nixfmt/Predoc.dyn_o )
src/Nixfmt/Predoc.hs:169:35: warning: [GHC-63394] [-Wx-partial]
In the use of ‘head’
(imported from Prelude, but defined in GHC.Internal.List):
"This is a partial function, it throws an error on empty lists. Use pattern matching, 'Data.List.uncons' or 'Data.Maybe.listToMaybe' instead. Consider refactoring to use "Data.List.NonEmpty"."
|
169 | if p /= [] && (isSoftSpacing (head p) || isSoftSpacing (last p))
| ^^^^
src/Nixfmt/Predoc.hs:634:27: warning: [GHC-63394] [-Wx-partial]
In the use of ‘head’
(imported from Prelude, but defined in GHC.Internal.List):
"This is a partial function, it throws an error on empty lists. Use pattern matching, 'Data.List.uncons' or 'Data.Maybe.listToMaybe' instead. Consider refactoring to use "Data.List.NonEmpty"."
|
634 | grp' = case head grp of
| ^^^^
src/Nixfmt/Predoc.hs:635:30: warning: [GHC-63394] [-Wx-partial]
In the use of ‘tail’
(imported from Prelude, but defined in GHC.Internal.List):
"This is a partial function, it throws an error on empty lists. Replace it with 'drop' 1, or use pattern matching or 'GHC.Internal.Data.List.uncons' instead. Consider refactoring to use "Data.List.NonEmpty"."
|
635 | Spacing _ -> tail grp
| ^^^^
src/Nixfmt/Predoc.hs:636:70: warning: [GHC-63394] [-Wx-partial]
In the use of ‘tail’
(imported from Prelude, but defined in GHC.Internal.List):
"This is a partial function, it throws an error on empty lists. Replace it with 'drop' 1, or use pattern matching or 'GHC.Internal.Data.List.uncons' instead. Consider refactoring to use "Data.List.NonEmpty"."
|
636 | Group ann ((Spacing _) : inner) -> Group ann inner : tail grp
| ^^^^
```
The treefmt parallel execution fix has been merged upstream, so the fetchpatch overlay is no longer needed. Also remove upper bounds in `nixfmt.cabal` AFAICT, all we've ever done with these upper bounds is bump them. They're failing now that we've bumped nixpkgs. Rather than continue to kick the can, just remove them. Justification: bounds like this make sense when you're a library in a semver ecosystem. Others need to be able to link against you, ideally without being forced to use the exact same version of all the things you depend on. However, we are *not* a Haskell library (haskell is just an implementation detail of the cli binary we build). We get reproducibility by virtue of using the nixpkgs Haskell package set. When we update nixpkgs, we rely upon our tests to tell us if anything broke. Co-authored-by: Jeremy Fleischman <me@jfly.fyi>
With the updates came a new version of fourmolu. Here are the changes.
To address this warning: > evaluation warning: nixfmt-rfc-style is now the same as pkgs.nixfmt which should be used instead.
I was seeing this warning: ```console $ nix-build -A checks.reuse /nix/store/7vlrdv22ai5ajs2p798md5a13g3j7p8q-python3.13-reuse-6.2.0/lib/python3.13/site-packages/reuse/project.py:332: PendingDeprecationWarning: '.reuse/dep5' is deprecated. You are recommended to instead use REUSE.toml. Use `reuse convert-dep5` to convert. ``` I fixed it by running `nix run --inputs-from . nixpkgs#reuse convert-dep5` at the root of the repo.
I noticed that the newer version of `cabal check` that we have now has a `--ignore` option, which lets us use ignore the warnings about missing dependency bounds and the `werror` flag, I included justification inline. We had the following errors which I did address: ``` Error: [git-protocol] Cloning over git:// might lead to an arbitrary code execution vulnerability. Furthermore, popular forges like GitHub do not support it. Use https:// or ssh:// instead. These warnings will likely cause trouble when distributing the package: Warning: [no-maintainer] No 'maintainer' field. These warnings may cause trouble when distributing the package: Warning: [doc-place] Please consider moving the file 'CHANGELOG.md' from the 'extra-source-files' section of the .cabal file to the section 'extra-doc-files'. Error: Hackage would reject this package. ``` I added this check to ci, so we won't regress in the future.
Collaborator
Author
|
I hadn't realized that our |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Note: There are multiple commits in here, I suggest reading them one at a time.
Note: this PR depends on #397. Please review/merge that PR before this one!