fish: 3.7.1 -> 4.0 #367229

r-vdp · 2024-12-21T22:38:54Z

Things done

https://github.com/fish-shell/fish-shell/releases/tag/4.0b1

This PR will need to get updated when the final version gets released, but I wanted to already try it out.

Add a 👍 reaction to pull requests you find important.

r-vdp · 2024-12-22T13:43:50Z

@ofborg build fish fish.passthru.tests

pkgs/shells/fish/default.nix

Eveeifyeve · 2024-12-24T08:27:22Z

pkgs/shells/fish/default.nix

    pname = "fish";
-    version = "3.7.1";
+    version = "4.0b1";

    src = fetchurl {


Can't you use a fetchFromGitHub?

Did you read the comment just below, telling you why FFGH isn't used but instead the release tarball is fetched manually?

Though as this is a major release, we should verify if this difference still exists between source code from the repo and released source code.

I think this is a good opportunity to switch to ffgh for better security. The fish version can be set as an env var https://codeberg.org/adamcstephens/nix-sandbox/src/commit/ad75d616bebe5b52a2178bc24151b500846b41e4/packages/fish/package.nix#L39

If the documentation note still exists we should definitely investigate that, but I’d feel much more comfortable using the git source.

Yeah, I'll have a look at that

If you fetch from github, you'll have to generate the docs yourself, which needs sphinx.

Right, I'll have to check how that's done then.

In terms of upstream: Install sphinx before you run cmake.

No idea if nix needs anything special.

Install sphinx before you run cmake.

Also known as just putting sphinx in nativeBuildInputs and hoping cmake picks it up.

(ideally put the fetchFromGithub and sphinx improvements in separate commits)

There was a bit more to it. I ran into an issue with fish_indent_lexer, and I'm not sure why upstream didn't run into this. I'll send the patch upstream in any case to see what they say.

pkgs/shells/fish/default.nix

Eveeifyeve · 2024-12-24T08:38:54Z

Again on line 439 where the test is an another IFD which is against nixpkgs.

r-vdp · 2024-12-24T10:25:53Z

@Eveeifyeve I'm sorry but your review comments make no sense. You either need to get more familiar with nix and nixpkgs before you start reviewing PRs, or you are simply a troll.

I am kindly asking you to refrain from further review comments on this PR. If you do continue despite this, I will report you to the moderation team.

SigmaSquadron

Thank you for this! I've used it for a few days, and haven't had any issues so far. The diff looks good as well, and the fixed tests pass.

Eveeifyeve · 2024-12-24T12:21:23Z

@Eveeifyeve I'm sorry but your review comments make no sense. You either need to get more familiar with nix and nixpkgs before you start reviewing PRs, or you are simply a troll.

I am not a troll, but I just didn't know if it was IFD or not.

I am kindly asking you to refrain from further review comments on this PR. If you do continue despite this, I will report you to the moderation team.

Ok sounds good, I will report this to them just to let them know that I didn't know if this IFD or not.

r-vdp · 2024-12-24T12:39:38Z

Ok sounds good, I will report this to them just to let them know that I didn't know if this IFD or not.

No need, I didn't do anything.

r-vdp · 2024-12-28T22:59:08Z

There seem to be some flaky tests that fail from time to time on my machine, so we might need to exclude those still.

charlottia · 2024-12-29T08:23:31Z

Thanks so much for this! I started making an attempt at this myself last week and promptly gave up when it started getting hairy; working great for me :)

NixOS/nixpkgs#367229

Lillecarl · 2024-12-29T19:05:36Z

Thanks for putting in the work. I was looking into this myself and got stuck on how CMake and cargo interact. It was surprisingly elegant.

Considering the scope of 3.7.1 -> 4, would it make sense to keep 3.7.1 around until a (couple?) 4.x.x fixing possible regressions? I think we could get 4 merged sooner if it doesn't replace 3.7.1?

[lillecarl@hetzner1] in [☸ default (valkey)]~/C/nixos [🎋 master][!][🐍 v3.12.8][🗀 loaded/allowed][🐚fish]
[20:01:25]❯ echo $FISH_VERSION
4.0b1

😄

adamcstephens · 2024-12-29T19:36:20Z

Considering the scope of 3.7.1 -> 4, would it make sense to keep 3.7.1 around until a (couple?) 4.x.x fixing possible regressions?

I'm personally opposed to maintaining two versions. It requires extra effort that I'm not convinced is worth it in this case. I've been using fish from git for many months, including now this PR. Besides a few intended behavior changes by upstream it's a drop in replacement.

If you're concerned about issues, I'd encourage trying this version and providing feedback upstream.

When 4.0 final is released I'd like to move it into unstable. We clearly won't backport this to stable.

pkgs/shells/fish/default.nix

will · 2025-01-02T19:32:58Z

FWIW, this branch built for me with no issues at all on my aarch64-darwin. I didnt do a full nixpkgs-review run, just nix build "github:nixos/nixpkgs?ref=fish#fish"

❯ nix build "github:nixos/nixpkgs?ref=fish#fish"
❯ result/bin/fish --version
fish, version 4.0b1
❯ result/bin/fish
❯ echo "whoo"
whoo

oxalica · 2025-01-14T01:53:52Z

The latest commit (d458fc2) sometimes fails to build for me. It passes on the second run. The history test is also known to be flaky in fish-shell/fish-shell#8789. Maybe we could disable it?

(nix build --rebuild can be used to test rebuild even with path existing.)

fish> pexpects/history.py..FAILED (5207 ms)

fish> 184/187 Test #168: history.py ...............................***Failed    5.27 sec
fish> pexpects/history.py..FAILED (5207 ms)
fish> Failed to match pattern: prompt 16
fish> history.py:101: timeout from expect_prompt("history delete -e -C 'echo hello'" + TO_END_SUFFIX)
fish> Escaped buffer:
fish> When written to the tty, this looks like:
fish> <-------
fish> (no trailing newline)------->
fish> Last 10 messages:
fish> OUTPUT      29.83 ms (Line 87): echo hello\r\n
fish> OUTPUT      +0.45 ms (Line 87): prompt 13>
fish>  INPUT      +0.28 ms (Line 91): history search --exact 'echo hell' | cat\n
fish> OUTPUT      +0.62 ms (Line 92): history search --exact 'echo hell'
fish> OUTPUT      +2.08 ms (Line 92): prompt 14>
fish>  INPUT      +0.25 ms (Line 95): history search --prefix 'echo start*echo end' | cat\n
fish> OUTPUT      +2.29 ms (Line 96): echo start1; builtin history; echo end1\r\n
fish> OUTPUT      +0.48 ms (Line 96): prompt 15>
fish>  INPUT      +0.20 ms (Line 100): history delete -e -C 'echo hello'\n
fish> OUTPUT      +1.40 ms (Line 101): history delete -e -C 'echo hello'\rprompt 15>history delete -e -C 'echo hello'\x1b]133;C\x07\x1b[?2004l\x1b[>4;0m\x1b>\x1b]133;D;0\x07\x1b[?25h⏎                                                                              \r⏎ \r\rprompt 16>\x1b[?2004h\x1b[>4;1m\x1b=
fish> Tmpdir is /build/fishtest-69ukorqn
fish> Failed tests:
fish>     pexpects/history.py
fish> 185/187 Test #178: signals.py ...............................   Passed    4.72 sec
fish> 186/187 Test #154: bind_mode_events.py ......................   Passed   40.29 sec
fish> 187/187 Test #187: cargo-test ...............................   Passed   50.95 sec
fish> 99% tests passed, 1 tests failed out of 187
fish> Total Test time (real) =  55.27 sec
fish> The following tests did not run:
fish>    63 - git.fish (Skipped)
fish>    92 - print-help.fish (Skipped)
fish>   125 - tmux-abbr.fish (Skipped)
fish>   126 - tmux-autosuggestion.fish (Skipped)
fish>   127 - tmux-bind.fish (Skipped)
fish>   128 - tmux-bind2.fish (Skipped)
fish>   129 - tmux-commandline.fish (Skipped)
fish>   130 - tmux-complete.fish (Skipped)
fish>   131 - tmux-complete2.fish (Skipped)
fish>   132 - tmux-history-search.fish (Skipped)
fish>   133 - tmux-job.fish (Skipped)
fish>   134 - tmux-multiline-prompt.fish (Skipped)
fish>   135 - tmux-prompt.fish (Skipped)
fish>   136 - tmux-scrollback.fish (Skipped)
fish>   137 - tmux-signal.fish (Skipped)
fish>   138 - tmux-transient.fish (Skipped)
fish> The following tests FAILED:
fish>   168 - history.py (Failed)
fish> Errors while running CTest
fish> FAILED: CMakeFiles/fish_run_tests /build/source/build/CMakeFiles/fish_run_tests
fish> cd /build/source/build && env CTEST_PARALLEL_LEVEL=14 FISH_FORCE_COLOR=1 FISH_SOURCE_DIR=/build/source /nix/store/38cffsqqx823crf1i4bcf6zz1qz1hgpd-cmake-3.30.5/bin/ctest --force-new-ctest-process --output-on-failure --progress
fish> ninja: build stopped: subcommand failed.

Another thing I noticed is that, the default output out also contains man, pkgconfig, which can be split into man and dev outputs. But the benefit is quite small, because they are only <5% of the total size. They are previously packaged as is, so I don't have strong opinion about it.

Eveeifyeve · 2025-01-14T05:39:32Z

I've also have a lot of applications fail when running nixpkgs-review is this normal?

r-vdp · 2025-01-14T09:42:06Z

I've also have a lot of applications fail when running nixpkgs-review is this normal?

I only see a test failure in kitty, and another test failure in a fish plugin (pure), so I definitely wouldn't qualify that as "a lot".
For both we'd need to either make an issue upstream so that they fix their tests to be compatible (or they can make an issue against fish if these are actual regressions), or we can simply disable those tests in nixpkgs.

r-vdp · 2025-01-14T09:44:11Z

The latest commit (d458fc2) sometimes fails to build for me. It passes on the second run. The history test is also known to be flaky in fish-shell/fish-shell#8789. Maybe we could disable it?

(nix build --rebuild can be used to test rebuild even with path existing.)
fish> pexpects/history.py..FAILED (5207 ms)

Another thing I noticed is that, the default output out also contains man, pkgconfig, which can be split into man and dev outputs. But the benefit is quite small, because they are only <5% of the total size. They are previously packaged as is, so I don't have strong opinion about it.

I've been rebuilding in a loop, but didn't get any failure so far. It may depend on hardware, many of these tests seem to be flaky and can time out on machines with heavy load. I think we can just disable the test.

faho · 2025-01-14T09:53:30Z

Note: This has been synced to master.

That's not going to be included in 4.0, which is being prepared in the Integration_4.0.0 branch.

r-vdp · 2025-01-14T10:16:44Z

I've also have a lot of applications fail when running nixpkgs-review is this normal?

Note: This has been synced to master.

That's not going to be included in 4.0, which is being prepared in the Integration_4.0.0 branch.

@faho: oh, I see. Will the recent fixes from master still be cherry-picked into the integration branch then? Like for instance the zellij bug for which a workaround was committed yesterday.

faho · 2025-01-14T10:23:12Z

Will the recent fixes from master still be cherry-picked into the integration branch then?

Yes, that's the plan.

Like for instance the zellij bug for which a workaround was committed yesterday.

That bug wasn't in 4.0 to begin with (it's triggered by querying that we don't do in 4.0).

github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Dec 21, 2024

r-vdp requested review from adamcstephens, cole-h, winterqt and SigmaSquadron December 21, 2024 22:39

github-actions bot added 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 labels Dec 21, 2024

r-vdp force-pushed the fish branch from 955059f to 1523abe Compare December 22, 2024 12:35

NickCao reviewed Dec 22, 2024

View reviewed changes

pkgs/shells/fish/default.nix Outdated Show resolved Hide resolved

r-vdp force-pushed the fish branch 3 times, most recently from 50860a6 to b1740c3 Compare December 22, 2024 23:03

Eveeifyeve reviewed Dec 24, 2024

View reviewed changes

pkgs/shells/fish/default.nix Show resolved Hide resolved

Eveeifyeve reviewed Dec 24, 2024

View reviewed changes

pkgs/shells/fish/default.nix Show resolved Hide resolved

SigmaSquadron approved these changes Dec 24, 2024

View reviewed changes

SigmaSquadron added 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels Dec 24, 2024

r-vdp force-pushed the fish branch from b1740c3 to 84ad1a2 Compare December 25, 2024 13:10

ofborg bot requested a review from SigmaSquadron December 25, 2024 17:41

Lillecarl added a commit to Lillecarl/nixos that referenced this pull request Dec 29, 2024

fish: Use 4.0b1

bc3ac40

NixOS/nixpkgs#367229

emilazy mentioned this pull request Dec 30, 2024

gimp: 2.10.38 → 3.0.0-RC2 #67576

Draft

UnsolvedCypher reviewed Dec 31, 2024

View reviewed changes

pkgs/shells/fish/default.nix Show resolved Hide resolved

UnsolvedCypher approved these changes Dec 31, 2024

View reviewed changes

oxalica reviewed Jan 2, 2025

View reviewed changes

pkgs/shells/fish/default.nix Outdated Show resolved Hide resolved

r-vdp force-pushed the fish branch 2 times, most recently from 52e8fc8 to d458fc2 Compare January 13, 2025 23:27

r-vdp force-pushed the fish branch from d458fc2 to edf682b Compare January 14, 2025 09:56

r-vdp force-pushed the fish branch from edf682b to 420cc2d Compare January 15, 2025 13:28

r-vdp added 11 commits January 20, 2025 10:56

fish: remove unneeded config from test, this was fixed in nixpkgs

9521f87

fish: use runTest for the nixos test

f39a9b2

fish: move to by-name

c911eb7

fish: 3.7.1 -> 4.0

3400c0a

fish: use finalAttrs

749798e

fish: fix tests

f897017

fixup! fish: fix tests

3e893fd

fish: generate the documentation from source

bdb0b98

fish: replace usages of sed by substituteInPlace

58c125e

fish: don't generate man pages in the test VM

7a3187e

fish: sync with the to-be-released branch

cc5d486

r-vdp force-pushed the fish branch from 420cc2d to cc5d486 Compare January 20, 2025 10:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fish: 3.7.1 -> 4.0 #367229

fish: 3.7.1 -> 4.0 #367229

r-vdp commented Dec 21, 2024 •

edited

Loading

r-vdp commented Dec 22, 2024

Eveeifyeve Dec 24, 2024

NobbZ Dec 24, 2024

adamcstephens Dec 24, 2024 •

edited

Loading

adamcstephens Dec 24, 2024

r-vdp Dec 24, 2024

faho Dec 25, 2024

r-vdp Dec 25, 2024

faho Dec 25, 2024

SigmaSquadron Dec 25, 2024 •

edited

Loading

r-vdp Dec 28, 2024

Eveeifyeve commented Dec 24, 2024

r-vdp commented Dec 24, 2024

SigmaSquadron left a comment •

edited

Loading

Eveeifyeve commented Dec 24, 2024

r-vdp commented Dec 24, 2024

r-vdp commented Dec 28, 2024

charlottia commented Dec 29, 2024

Lillecarl commented Dec 29, 2024

adamcstephens commented Dec 29, 2024

will commented Jan 2, 2025

oxalica commented Jan 14, 2025

Eveeifyeve commented Jan 14, 2025

r-vdp commented Jan 14, 2025 •

edited

Loading

r-vdp commented Jan 14, 2025

faho commented Jan 14, 2025

r-vdp commented Jan 14, 2025

faho commented Jan 14, 2025

fish: 3.7.1 -> 4.0 #367229

Are you sure you want to change the base?

fish: 3.7.1 -> 4.0 #367229

Conversation

r-vdp commented Dec 21, 2024 • edited Loading

Things done

r-vdp commented Dec 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamcstephens Dec 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SigmaSquadron Dec 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Eveeifyeve commented Dec 24, 2024

r-vdp commented Dec 24, 2024

SigmaSquadron left a comment • edited Loading

Choose a reason for hiding this comment

Eveeifyeve commented Dec 24, 2024

r-vdp commented Dec 24, 2024

r-vdp commented Dec 28, 2024

charlottia commented Dec 29, 2024

Lillecarl commented Dec 29, 2024

adamcstephens commented Dec 29, 2024

will commented Jan 2, 2025

oxalica commented Jan 14, 2025

Eveeifyeve commented Jan 14, 2025

r-vdp commented Jan 14, 2025 • edited Loading

r-vdp commented Jan 14, 2025

faho commented Jan 14, 2025

r-vdp commented Jan 14, 2025

faho commented Jan 14, 2025

r-vdp commented Dec 21, 2024 •

edited

Loading

adamcstephens Dec 24, 2024 •

edited

Loading

SigmaSquadron Dec 25, 2024 •

edited

Loading

SigmaSquadron left a comment •

edited

Loading

r-vdp commented Jan 14, 2025 •

edited

Loading