gdtoolkit_4: init at 4.2.2 by squarepear · Pull Request #291040 · NixOS/nixpkgs

squarepear · 2024-02-24T02:12:26Z

Description of changes

Rename gdtoolkit to gdtoolkit_3
Init gdtoolkit_4 at v4.2.2
Update gdtoolkit_3 to v3.5.0

Tested and seems to be working fine.

Resolves #267249

Things done

Add a 👍 reaction to pull requests you find important.

kirillrdy · 2024-02-24T06:01:52Z

Result of nixpkgs-review pr 291040 run on x86_64-linux 1

2 packages failed to build:

gdtoolkit_4
gdtoolkit_4.dist

2 packages built:

gdtoolkit_3
gdtoolkit_3.dist

logs

=============================== warnings summary ===============================
gdtoolkit/parser/parser.py:8
  /build/source/gdtoolkit/parser/parser.py:8: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
    import pkg_resources

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================== short test summary info ============================
FAILED tests/generated/test_expression_parsing.py::test_expression_formatting - gdtoolkit.formatter.exceptions.TreeInvariantViolation: TreeInvariantViolati...
===== 1 failed, 497 passed, 307 skipped, 2 deselected, 1 warning in 44.31s =====

kirillrdy · 2024-02-24T06:36:13Z

it build the second time :-)

Result of nixpkgs-review pr 291040 run on x86_64-linux 1

4 packages built:

gdtoolkit_3
gdtoolkit_3.dist
gdtoolkit_4
gdtoolkit_4.dist

squarepear · 2024-03-07T19:45:54Z

I just had the same issue when running on GitHub actions, it was fine the second time I ran it? I'll look into it, but it's a bit difficult since it doesn't happen locally or even consistently.

Some quick research makes me think it is something to do with setuptools, maybe it should be somewhere else rather than propagatedBuildInputs? Still very strange that it isn't always an issue.

squarepear · 2024-03-13T16:14:53Z

I've been using the package for the past week or so without running in to the issue. setuptools had been updated in nixpkgs since creating this PR, so I updated the branch. I believe there was an issue with that specific version of setuptools that has since been fixed.

I think this PR is ready to merge!

kirillrdy · 2024-03-19T09:52:05Z

pkgs/by-name/gd/gdtoolkit_3/package.nix

https://github.com/NixOS/nixpkgs/blob/master/pkgs/README.md#package-naming

The pname attribute should be identical to the upstream package name.

Since both versions of gdtoolkit may be in use, I followed what was chosen for Godot 3 and 4. There may be a better way to do this that I am missing, so please let me know if that is the case!

nixpkgs/pkgs/development/tools/godot/3/default.nix

Line 29 in 614b461

pname = "godot3";

nixpkgs/pkgs/development/tools/godot/4/default.nix

Line 45 in 614b461

pname = "godot";

version will be appended, so you can have both installed name=godot-3.3.1 and name=godot-4.4.2

I am not aware of any issues or reasons for changing pname

Hmm, I originally thought that as well, but wanted to keep it similar to Godot. I'll check to see if I can find an issue/PR where that was discussed to see why that was chosen.

It seems the names should be without an _ then? That's what godot itself is doing, and seems nice to have it consistent.

The docs do indeed say The pname attribute should be identical to the upstream package name. (as linked above) but should is key here, and this feels like a valid reason for the extra number.

kirillrdy · 2024-03-19T09:52:36Z

pkgs/top-level/aliases.nix

whats the justification for now having this toplevel attribute ?

Like with the package name, I followed how this was implemented for Godot 3 and 4.

nixpkgs/pkgs/top-level/aliases.nix

Line 327 in 614b461

godot = throw "godot has been renamed to godot3 to distinguish from version 4"; # Added 2023-07-16

TheOddler

Tested and works for me. Code seems good. Do see my comment below about the naming, I'm happy to approve already and leave it up to your best judgement with the name.

TheOddler · 2024-05-21T21:26:54Z

pkgs/by-name/gd/gdtoolkit_3/package.nix

It seems the names should be without an _ then? That's what godot itself is doing, and seems nice to have it consistent.

The docs do indeed say The pname attribute should be identical to the upstream package name. (as linked above) but should is key here, and this feels like a valid reason for the extra number.

squarepear · 2024-05-21T21:41:03Z

That's fair, I can get that updated! The whole naming scheme with Godot is a bit odd, godot3 vs godot_4, but it makes sense to match the pnames.

TheOddler · 2024-05-21T22:01:37Z

Yea, I got confused looking into the naming too, the Godot packages don't seem consistent, and they changed it for godot 3 but not for godot 4 at some point, idk. A bit messy, so I guess best you can do is not add to the mess?

kirillrdy · 2024-06-04T08:37:46Z

Result of nixpkgs-review pr 291040 run on x86_64-linux 1

2 packages failed to build:

gdtoolkit_4
gdtoolkit_4.dist

2 packages built:

gdtoolkit_3
gdtoolkit_3.dist

TheOddler · 2024-06-04T09:20:50Z

Oh, I only tested by actually running it as part of my system (actually using it). Also tried nixpkgs-review now, and it fails some tests:

❯ nix-shell -p nixpkgs-review --run "nixpkgs-review pr 291040"
[...]
error: builder for '/nix/store/9s9kzlm9vj39inkznyb64i5hhaamj94j-gdtoolkit-4.2.2.drv' failed with exit code 1;
       last 10 log lines:
       > gdtoolkit/parser/parser.py:8
       >   /build/source/gdtoolkit/parser/parser.py:8: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
       >     import pkg_resources
       >
       > -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
       > =========================== short test summary info ============================
       > FAILED tests/generated/test_expression_parsing.py::test_expression_parsing - hypothesis.errors.InvalidArgument: Undefined terminals ['_DEDENT', '_INDENT...
       > FAILED tests/generated/test_expression_parsing.py::test_expression_formatting - hypothesis.errors.InvalidArgument: Undefined terminals ['_DEDENT', '_INDENT...
       > ===== 2 failed, 496 passed, 307 skipped, 2 deselected, 1 warning in 11.69s =====
       > /nix/store/xfhkjnpqjwlf6hlk1ysmq3aaq80f3bjj-stdenv-linux/setup: line 1579: pop_var_context: head of shell_variables not a function context
       For full logs, run 'nix log /nix/store/9s9kzlm9vj39inkznyb64i5hhaamj94j-gdtoolkit-4.2.2.drv'.
error: 1 dependencies of derivation '/nix/store/7jxp9pnzc5r05b8bizxxa6aqcnx56czv-review-shell.drv' failed to build

Link to currently reviewing PR:
https://github.com/NixOS/nixpkgs/pull/291040

2 packages failed to build:
gdtoolkit_4 gdtoolkit_4.dist

2 packages built:
gdtoolkit_3 gdtoolkit_3.dist

ArtixBTW · 2024-06-06T16:11:59Z

Oh, I only tested by actually running it as part of my system (actually using it). Also tried nixpkgs-review now, and it fails some tests:

❯ nix-shell -p nixpkgs-review --run "nixpkgs-review pr 291040"
[...]
error: builder for '/nix/store/9s9kzlm9vj39inkznyb64i5hhaamj94j-gdtoolkit-4.2.2.drv' failed with exit code 1;
       last 10 log lines:
       > gdtoolkit/parser/parser.py:8
       >   /build/source/gdtoolkit/parser/parser.py:8: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
       >     import pkg_resources
       >
       > -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
       > =========================== short test summary info ============================
       > FAILED tests/generated/test_expression_parsing.py::test_expression_parsing - hypothesis.errors.InvalidArgument: Undefined terminals ['_DEDENT', '_INDENT...
       > FAILED tests/generated/test_expression_parsing.py::test_expression_formatting - hypothesis.errors.InvalidArgument: Undefined terminals ['_DEDENT', '_INDENT...
       > ===== 2 failed, 496 passed, 307 skipped, 2 deselected, 1 warning in 11.69s =====
       > /nix/store/xfhkjnpqjwlf6hlk1ysmq3aaq80f3bjj-stdenv-linux/setup: line 1579: pop_var_context: head of shell_variables not a function context
       For full logs, run 'nix log /nix/store/9s9kzlm9vj39inkznyb64i5hhaamj94j-gdtoolkit-4.2.2.drv'.
error: 1 dependencies of derivation '/nix/store/7jxp9pnzc5r05b8bizxxa6aqcnx56czv-review-shell.drv' failed to build

Link to currently reviewing PR:
https://github.com/NixOS/nixpkgs/pull/291040

2 packages failed to build:
gdtoolkit_4 gdtoolkit_4.dist

2 packages built:
gdtoolkit_3 gdtoolkit_3.dist

I've been using this locally and worked around this by just disabling tests for these.

+  disabledTestPaths = [
+    "tests/generated/test_expression_parsing.py"
+    "tests/gdradon/test_executable.py"
+  ];

What I find odd though is that if I do multiple rebuilds these tests sometimes don't fail and sometimes do, quite odd, also not quite sure why disabledTests isn't disabling the individual tests that seem to be failing.

TheOddler · 2024-06-07T14:39:51Z

I've been using this locally

This is what I've been doing as well, and it works well so far, no problems.

worked around this by just disabling tests for these

I did not have to disable any tests. I'm not sure what your setup is, but I added github:squarepear/nixpkgs/gdtoolkit-4 as an input to my nixos config inputs (I'm using flakes), added it as an overlay to my channels, and then included gdtoolkit_4 in my system packages. Didn't have to do anything else like disabling tests.

squarepear · 2024-06-07T19:49:47Z

I've been using this locally and worked around this by just disabling tests for these.
+  disabledTestPaths = [
+    "tests/generated/test_expression_parsing.py"
+    "tests/gdradon/test_executable.py"
+  ];

It seems to be working for me, but the inconsistency makes it difficult to tell. Hopefully everything is good now, but if anyone still has issues, let me know!

kirillrdy

Result of nixpkgs-review pr 291040 run on x86_64-linux 1

4 packages built:

gdtoolkit_3
gdtoolkit_3.dist
gdtoolkit_4
gdtoolkit_4.dist

nixos-discourse · 2024-06-09T07:10:41Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/how-to-introduce-a-breaking-version-bump/46746/2

Thiago-Assis-T · 2024-06-09T13:05:12Z

Hello, this seemed to break my system upgrade, on unstable-small, even though i don't use godot...

it spits out on the upgrade:

         at /nix/store/ab6iajix47qdm2v098dcdaqis6094300-source/pkgs/top-level/aliases.nix:38:20:

           37|   # Make sure that we are not shadowing something from all-packages.nix.
           38|   checkInPkgs = n: alias:
             |                    ^
           39|     if builtins.hasAttr n super

       error: gdtoolkit has been renamed to gdtoolkit_3 to distinguish from version 4

this host is a media server, running jellyfin, radarr, deluge... nothing with godot or games, or game dev... not even a gui.

here is the config: https://github.com/Thiago-Assis-T/.nixos and the hostname is ThiagoServer, i tried show trace and mentioned the issue on discourse

nixos-discourse · 2024-06-09T13:06:05Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/package-name-changed-gdtoolkit-gdtoolkit-3-but-i-dont-use-that-package-directly-and-was-unable-to-identify-what-it-is-a-dep-of/46761/3

squarepear · 2024-06-09T13:17:16Z

Hello, this seemed to break my system upgrade, on unstable-small, even though i don't use godot...

it spits out on the upgrade:
         at /nix/store/ab6iajix47qdm2v098dcdaqis6094300-source/pkgs/top-level/aliases.nix:38:20:

           37|   # Make sure that we are not shadowing something from all-packages.nix.
           38|   checkInPkgs = n: alias:
             |                    ^
           39|     if builtins.hasAttr n super

       error: gdtoolkit has been renamed to gdtoolkit_3 to distinguish from version 4
this host is a media server, running jellyfin, radarr, deluge... nothing with godot or games, or game dev... not even a gui.

here is the config: Thiago-Assis-T/.nixos and the hostname is ThiagoServer, i tried show trace and mentioned the issue on discourse

That's very strange. I have a bit of time this morning, I'll look into it and see if I can find anything!

My first complete guess would be something like nvim or similar using it as a formatter, but it doesn't make much sense that you would have the gdtoolkit formatter in your config.

Thiago-Assis-T · 2024-06-09T13:20:12Z

AAAAA yess, i'm using nixvim, with all ts grammars... perhaps thats it? i dont have as formatter or as linter though, never messed with godot, so never added it...

squarepear · 2024-06-09T13:20:35Z

Wow, guess seems to have been correct! It looks like it is an issue with nixvim

nix-community/nixvim#1660
nix-community/nixvim#1659

squarepear · 2024-06-09T13:21:04Z

AAAAA yess, i'm using nixvim, with all ts grammars... perhaps thats it? i dont have as formatter or as linter though, never messed with godot, so never added it...

Yep 😄

Thiago-Assis-T · 2024-06-09T13:22:12Z

thank you @squarepear

nixos-discourse · 2024-06-09T13:22:46Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/package-name-changed-gdtoolkit-gdtoolkit-3-but-i-dont-use-that-package-directly-and-was-unable-to-identify-what-it-is-a-dep-of/46761/4

addressing: NixOS/nixpkgs#291040

squarepear force-pushed the gdtoolkit-4 branch from a134877 to 38d7eef Compare February 24, 2024 02:33

ofborg bot added 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (new) This PR adds a new package labels Feb 24, 2024

ofborg bot requested review from hesiod and shiryel February 24, 2024 02:36

ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 24, 2024

squarepear force-pushed the gdtoolkit-4 branch 4 times, most recently from 2c60700 to ec195a2 Compare March 7, 2024 22:18

kirillrdy reviewed Mar 19, 2024

View reviewed changes

TheOddler approved these changes May 21, 2024

View reviewed changes

squarepear force-pushed the gdtoolkit-4 branch from ec195a2 to bb663f8 Compare May 21, 2024 22:00

TheOddler approved these changes Jun 4, 2024

View reviewed changes

squarepear added 3 commits June 7, 2024 15:40

gdtoolkit_4: init at 4.2.2

64494ff

gdtoolkit: rename to gdtoolkit_3

c01927f

gdtoolkit_3: 3.3.1 -> 3.5.0

471f48a

squarepear force-pushed the gdtoolkit-4 branch from bb663f8 to 471f48a Compare June 7, 2024 19:42

kirillrdy approved these changes Jun 7, 2024

View reviewed changes

kirillrdy merged commit 6919189 into NixOS:master Jun 8, 2024

squarepear deleted the gdtoolkit-4 branch June 9, 2024 13:13

squarepear restored the gdtoolkit-4 branch June 9, 2024 13:13

squarepear deleted the gdtoolkit-4 branch June 11, 2024 23:06

nbw added a commit to nbw/nixvim that referenced this pull request Jun 23, 2024

update lock file

e9c1fca

addressing: NixOS/nixpkgs#291040

michaelhthomas mentioned this pull request Jun 27, 2024

[BUG] gdtoolkit is now (gdtoolkit_3|gdtoolkit_4) nix-community/nixvim#1660

Closed

1 task

Uh oh!

Conversation

squarepear commented Feb 24, 2024

Description of changes

Things done

Uh oh!

kirillrdy commented Feb 24, 2024

Uh oh!

kirillrdy commented Feb 24, 2024

Uh oh!

squarepear commented Mar 7, 2024

Uh oh!

squarepear commented Mar 13, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheOddler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

squarepear commented May 21, 2024

Uh oh!

TheOddler commented May 21, 2024

Uh oh!

kirillrdy commented Jun 4, 2024

Uh oh!

TheOddler commented Jun 4, 2024

Uh oh!

ArtixBTW commented Jun 6, 2024

Uh oh!

TheOddler commented Jun 7, 2024

Uh oh!

squarepear commented Jun 7, 2024

Uh oh!

kirillrdy left a comment

Choose a reason for hiding this comment

Uh oh!

nixos-discourse commented Jun 9, 2024

Uh oh!

Thiago-Assis-T commented Jun 9, 2024

Uh oh!

nixos-discourse commented Jun 9, 2024

Uh oh!

squarepear commented Jun 9, 2024

Uh oh!

Thiago-Assis-T commented Jun 9, 2024

Uh oh!

squarepear commented Jun 9, 2024

Uh oh!

squarepear commented Jun 9, 2024

Uh oh!

Thiago-Assis-T commented Jun 9, 2024

Uh oh!

nixos-discourse commented Jun 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Comments