Skip to content

Conversation

@ngiger
Copy link
Contributor

@ngiger ngiger commented Feb 16, 2021

Motivation for this change

Add a ruby gem I use for importing FIT data after jogging

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • [x ] NixOS
    • macOS
    • other Linux distributions
  • [x ] Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

I am puzzled, that testing it locally does not work, as show by

nix-build $NIXPKGS -A postrunner
/nix/store/hir9dlqwcxzliakqwzvvmvmn1hhwvk3a-postrunner-1.0.4
result/bin/postrunner --version
Traceback (most recent call last):
9: from result/bin/postrunner:18:in `<main>'
8: from /nix/store/iwxp1n2manz98yrkznqmpfvq07h4nax6-bundler-2.1.4/lib/ruby/gems/2.6.0/gems/bundler-2.1.4/lib/bundler.rb:149:in `setup'
7: from /nix/store/iwxp1n2manz98yrkznqmpfvq07h4nax6-bundler-2.1.4/lib/ruby/gems/2.6.0/gems/bundler-2.1.4/lib/bundler/runtime.rb:20:in `setup'
6: from /nix/store/iwxp1n2manz98yrkznqmpfvq07h4nax6-bundler-2.1.4/lib/ruby/gems/2.6.0/gems/bundler-2.1.4/lib/bundler/runtime.rb:101:in `block in definition_method'
5: from /nix/store/iwxp1n2manz98yrkznqmpfvq07h4nax6-bundler-2.1.4/lib/ruby/gems/2.6.0/gems/bundler-2.1.4/lib/bundler/definition.rb:226:in `requested_specs'
4: from /nix/store/iwxp1n2manz98yrkznqmpfvq07h4nax6-bundler-2.1.4/lib/ruby/gems/2.6.0/gems/bundler-2.1.4/lib/bundler/definition.rb:237:in `specs_for'
3: from /nix/store/iwxp1n2manz98yrkznqmpfvq07h4nax6-bundler-2.1.4/lib/ruby/gems/2.6.0/gems/bundler-2.1.4/lib/bundler/definition.rb:170:in `specs'
2: from /nix/store/iwxp1n2manz98yrkznqmpfvq07h4nax6-bundler-2.1.4/lib/ruby/gems/2.6.0/gems/bundler-2.1.4/lib/bundler/spec_set.rb:80:in `materialize'
1: from /nix/store/iwxp1n2manz98yrkznqmpfvq07h4nax6-bundler-2.1.4/lib/ruby/gems/2.6.0/gems/bundler-2.1.4/lib/bundler/spec_set.rb:80:in `map!'
/nix/store/iwxp1n2manz98yrkznqmpfvq07h4nax6-bundler-2.1.4/lib/ruby/gems/2.6.0/gems/bundler-2.1.4/lib/bundler/spec_set.rb:86:in `block in materialize': Could not find bindata-2.4.8 in any of the sources (Bundler::GemNotFound)

Running the tests via nix-build nixos/tests/postrunner.nix shows not problem.
But this is the same with gollum (another ruby gem) I tested with.

@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Feb 16, 2021
@r-rmcgibbo
Copy link

r-rmcgibbo commented Feb 16, 2021

Result of nixpkgs-review pr 113317 at c38c471f run on aarch64-linux 1

1 package built:
  • postrunner
1 suggestion:
  • warning: unused-argument

    Unused argument: bundlerUpdateScript.
    Near pkgs/applications/misc/postrunner/default.nix:1:20:

      |
    1 | { lib, bundlerApp, bundlerUpdateScript }:
      |                    ^
    

Result of nixpkgs-review pr 113317 at c38c471f run on x86_64-linux 1

1 package built:
  • postrunner
1 suggestion:
  • warning: unused-argument

    Unused argument: bundlerUpdateScript.
    Near pkgs/applications/misc/postrunner/default.nix:1:20:

      |
    1 | { lib, bundlerApp, bundlerUpdateScript }:
      |                    ^
    

@ngiger ngiger force-pushed the postrunner branch 2 times, most recently from 3255898 to 55a3059 Compare February 16, 2021 17:38
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You forgot to add the gemfile which pins the version number.

@ngiger
Copy link
Contributor Author

ngiger commented Feb 18, 2021

Added Gemfile. Really a stupid error by me. Thanks a lot for the reviews.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we pin the version here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pinned the version there. But I think this violates an assumption of the bundler-update-script (see pkgs/development/ruby-modules/bundler-update-script/default.nix).
But bundler-update-script is not documented and I am still a novice NixOS user.
I would appreciate if @nicknovitski could answer this question.
I would suggest to add
in https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/ruby.section.md#packaging-executables-that-require-wrapping
a few lines to mention the purpose of bundler-update-script and when it is called. Probably somewhere via another script at a given point for each NixOS release.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote it to follow the convention that I saw in other packages and the manual (including in that linked section), where no version specification is added to the Gemfile, since committing the lockfile pins the version anyway.

@ngiger ngiger force-pushed the postrunner branch 2 times, most recently from 63a203d to 9847ceb Compare March 1, 2021 20:06
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the package init commit to postrunner: init at 1.0.4.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A "test" like this can be implemented as an installCheck.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but this is beyond my novice NixOS level.

As I understand, I should

  1. Remove the file nixos/tests/postrunner.nix and
  2. Add somehow a derivation of bundlerApp where I add some lines like
    doInstallCheck = true;
    installCheckPhase = ''
      if [[ "$out/bin/postrunner --version)" == "postrunner version 1.0.4" ]]; then
        echo 'postrunner smoke check passed'
      else
        echo 'postrunner smoke check failed'
        return 1
      fi
    '';

But I found no file under nixpks where a bundlerApp has a checkInstall. And I am not versed enough to make this happen.

Can you give me hint or a sketch of the solution?

Thanks in advance.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternately, you can use passthru.tests with runCommand helper, see https://nixos.org/manual/nixpkgs/unstable/#sec-package-tests

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/howto-run-a-postinstall-check-for-a-ruby-bundlerapp/13780/1

@ngiger
Copy link
Contributor Author

ngiger commented Jun 25, 2021

Thanks jtojnar for your tipp. But I thinks these kind of tests work for python and haskell, but not for ruby gem packaged via bundlerEnv or bundlerApp.
And adding the lines

 passthru.doInstallCheck = true;
  passthru.installCheckPhase = ''
   echo doInstallCheck is true
  '';

Did not produce any output neither.

@jtojnar
Copy link
Member

jtojnar commented Jun 25, 2021

The point of passthru.tests is that they are run separately (e.g. nix-build -A postrunner.tests) our CI runs them.

@github-actions github-actions bot removed the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Jun 25, 2021
@stale
Copy link

stale bot commented Jan 6, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 6, 2022
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Feb 5, 2022
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/720

@raboof
Copy link
Member

raboof commented Apr 27, 2022

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

1 package built:
  • postrunner

Copy link
Member

@raboof raboof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, one more thing to look at

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding was, that with the GPL v2 you are entitled to use it also with a later version of the GPL. But I am not a license specialist.

But on github pkgs/tools/text/unoconv/default.nix which has licenses.gpl2 points to same license GPL v20 as the gpl2only marked in pkgs/tools/typesetting/htmldoc/default.nix.

And in https://github.com/NixOS/nixpkgs/blob/master/lib/licenses.nix we have

 gpl2Only = {
    spdxId = "GPL-2.0-only";
    fullName = "GNU General Public License v2.0 only";
  };

I do not find any mentioning of GPL-2.0-only in the postrunner code
.
So I am confused. Do you now how can get a definitive answer?

Copy link
Member

@SuperSandro2000 SuperSandro2000 Apr 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding was, that with the GPL v2 you are entitled to use it also with a later version of the GPL. But I am not a license specialist.

If https://github.com/scrapper/postrunner/blob/master/lib/postrunner.rb#L8-L10 would mention or later. So gpl2Only is correct.

I do not find any mentioning of GPL-2.0-only in the postrunner code

Only is defined by the absence of or at your option any later version.

Copy link
Member

@raboof raboof Apr 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you now how can get a definitive answer?

I think the license text is pretty clear: https://github.com/scrapper/postrunner/blob/master/COPYING#L242

If the Program specifies a version number of this License which applies to it and "any later version", you have the option of following the terms and conditions either of that version or of any later version published by the Free Software Foundation. If the Program does not specify a version number of this License, you may choose any version ever published by the Free Software Foundation.

So AFAICS that confirms that, since they do specify 'version 2' but don't specify 'or any later version', we should interpret that as gpl2Only.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
homepage = "https://github.com/scrapper/postrunner.git";
homepage = "https://github.com/scrapper/postrunner";

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
license = licenses.gpl2only;
license = licenses.gpl2Only;

@raboof
Copy link
Member

raboof commented May 2, 2022

As we don't squash on merge, perhaps the last 3 commits could be squashed into one postrunner: init at 1.0.5 commit?

Copy link
Member

@raboof raboof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starting to look good, but postrunner version gives me:

ERROR: Permission denied @ rb_sysopen - /home/aengelen/.postrunner/html/icons/last.png
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:1415:in `initialize'
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:1415:in `open'
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:1415:in `block in copy_file'
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:1414:in `open'
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:1414:in `copy_file'
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:1379:in `copy'
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:497:in `block in copy_entry'
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:1513:in `wrap_traverse'
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:1516:in `block in wrap_traverse'
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:1515:in `each'
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:1515:in `wrap_traverse'
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:494:in `copy_entry'
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:468:in `block in cp_r'
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:1589:in `block in fu_each_src_dest'
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:1603:in `fu_each_src_dest0'
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:1587:in `fu_each_src_dest'
/nix/store/55ywcfngxkymn252792i5mg6zd8glaqb-ruby-2.7.6/lib/ruby/2.7.0/fileutils.rb:467:in `cp_r'
/nix/store/l962hj6g01y4xp8n4hdzh7jih5y4pw5z-ruby2.7.6-postrunner-1.0.5/lib/ruby/gems/2.7.0/gems/postrunner-1.0.5/lib/postrunner/ActivitiesDB.rb:339:in `create_auxdir'
/nix/store/l962hj6g01y4xp8n4hdzh7jih5y4pw5z-ruby2.7.6-postrunner-1.0.5/lib/ruby/gems/2.7.0/gems/postrunner-1.0.5/lib/postrunner/ActivitiesDB.rb:83:in `block in create_directories'
/nix/store/l962hj6g01y4xp8n4hdzh7jih5y4pw5z-ruby2.7.6-postrunner-1.0.5/lib/ruby/gems/2.7.0/gems/postrunner-1.0.5/lib/postrunner/ActivitiesDB.rb:82:in `each'
/nix/store/l962hj6g01y4xp8n4hdzh7jih5y4pw5z-ruby2.7.6-postrunner-1.0.5/lib/ruby/gems/2.7.0/gems/postrunner-1.0.5/lib/postrunner/ActivitiesDB.rb:82:in `create_directories'
/nix/store/l962hj6g01y4xp8n4hdzh7jih5y4pw5z-ruby2.7.6-postrunner-1.0.5/lib/ruby/gems/2.7.0/gems/postrunner-1.0.5/lib/postrunner/ActivitiesDB.rb:36:in `initialize'
/nix/store/l962hj6g01y4xp8n4hdzh7jih5y4pw5z-ruby2.7.6-postrunner-1.0.5/lib/ruby/gems/2.7.0/gems/postrunner-1.0.5/lib/postrunner/Main.rb:646:in `new'
/nix/store/l962hj6g01y4xp8n4hdzh7jih5y4pw5z-ruby2.7.6-postrunner-1.0.5/lib/ruby/gems/2.7.0/gems/postrunner-1.0.5/lib/postrunner/Main.rb:646:in `import_legacy_archive'
/nix/store/l962hj6g01y4xp8n4hdzh7jih5y4pw5z-ruby2.7.6-postrunner-1.0.5/lib/ruby/gems/2.7.0/gems/postrunner-1.0.5/lib/postrunner/Main.rb:311:in `execute_command'
/nix/store/l962hj6g01y4xp8n4hdzh7jih5y4pw5z-ruby2.7.6-postrunner-1.0.5/lib/ruby/gems/2.7.0/gems/postrunner-1.0.5/lib/postrunner/Main.rb:79:in `main'
/nix/store/l962hj6g01y4xp8n4hdzh7jih5y4pw5z-ruby2.7.6-postrunner-1.0.5/lib/ruby/gems/2.7.0/gems/postrunner-1.0.5/lib/postrunner.rb:24:in `<module:PostRunner>'
/nix/store/l962hj6g01y4xp8n4hdzh7jih5y4pw5z-ruby2.7.6-postrunner-1.0.5/lib/ruby/gems/2.7.0/gems/postrunner-1.0.5/lib/postrunner.rb:22:in `<top (required)>'
/nix/store/zgfbxq6bsvf35bx5870b3cizd9w8n4wl-postrunner-1.0.5/lib/ruby/gems/2.7.0/gems/postrunner-1.0.5/bin/postrunner:4:in `require'
/nix/store/zgfbxq6bsvf35bx5870b3cizd9w8n4wl-postrunner-1.0.5/lib/ruby/gems/2.7.0/gems/postrunner-1.0.5/bin/postrunner:4:in `<top (required)>'
/nix/store/fp9xmpa031yf4agzv04kicdnlkp4nj7d-postrunner-1.0.5/bin/postrunner:35:in `load'
/nix/store/fp9xmpa031yf4agzv04kicdnlkp4nj7d-postrunner-1.0.5/bin/postrunner:35:in `<main>'

*******************************************************************************
You have triggered a bug in PostRunner 1.0.5!

That does not seem good?

This file was just created by postrunner itself:

-r--r--r-- 1 aengelen users 1.2K Jul 29 17:18 /home/aengelen/.postrunner/html/icons/last.png

@ngiger
Copy link
Contributor Author

ngiger commented Jul 30, 2022

I cannot reproduce your problem. On my checkout I run

nix build -f default.nix pkgs.postrunner
result/bin/postrunner --version
1.0.5
result/bin/postrunner version
ERROR: Unknown command 'version'. See 'postrunner -h' for more information.

@raboof
Copy link
Member

raboof commented Jul 30, 2022

I cannot reproduce your problem. On my checkout I run

nix build -f default.nix pkgs.postrunner
result/bin/postrunner --version
1.0.5

That works for me as well

result/bin/postrunner version
ERROR: Unknown command 'version'. See 'postrunner -h' for more information.

This gives the Permission denied problem for me. I'm curious if nix-build -A postrunner.tests works for you - since that runs postrunner version it shouldn't succeed in this form, should it?

Anyway, since the documented behavior seems to work (and it's just the error reporting when using an invalid command that seems rather loud on my side) I guess this PR LGTM after fixing the test condition.

@ngiger
Copy link
Contributor Author

ngiger commented Jul 31, 2022

Running the tests works for me as well:

nix-build -A postrunner.tests
this derivation will be built:
/nix/store/wcy1bz1bz8xq9cgbz3x9krm95cz26zqi-postrunner-1.0.5-tests.drv
building '/nix/store/wcy1bz1bz8xq9cgbz3x9krm95cz26zqi-postrunner-1.0.5-tests.drv'...
Usage postrunner <command> [options]
All test for postrunner-1.0.5-tests passed
/nix/store/9iasxdx9pv75ki2jcdly833xjmarn90v-postrunner-1.0.5-tests

I have nix (Nix) 2.8.1

@ngiger ngiger requested a review from SuperSandro2000 July 31, 2022 00:30
@raboof
Copy link
Member

raboof commented Jul 31, 2022

Running the tests works for me as well:

Interesting - that is surprising since it does postrunner version instead of postrunner --version, right?

I have nix (Nix) 2.8.1

(I'm on 2.9.1, if that matters)

@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Dec 1, 2024
@wegank wegank added 2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md labels Dec 31, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 5, 2025
@raboof
Copy link
Member

raboof commented Jun 11, 2025

I tried rebasing this on nixos-unstable but now got:

html4_document.c: In function 'rb_html_document_s_read_io':
html4_document.c:49:52: error: passing argument 2 of 'xmlSetStructuredErrorFunc' from incompatible pointer type [-Wincompatible-pointer-types]

@ngiger
Copy link
Contributor Author

ngiger commented Jun 12, 2025

@raboof: I think my old way to add the postrunner gem is too complicated.
I think following https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/ruby.section.md it should be sufficient to add fit4ruby, perobs and postrunner into pkgs/top-level/ruby-packages.nix.
Then add postrunner in pkgs/development/ruby-modules/with-packages/Gemfile
This fails however as fit4ruby specifies bindata (~> 2.4.14), but nixpkgs has bindata 2.5.1.
I opened scrapper/fit4ruby#40 to fix this problem.
Once new versions of fit4ruby and postrunner are release, I will update this PR

@nixpkgs-ci nixpkgs-ci bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants