Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

steamtinkerlaunch: init at 11.11 #200157

Merged
merged 1 commit into from
Nov 29, 2022
Merged

steamtinkerlaunch: init at 11.11 #200157

merged 1 commit into from
Nov 29, 2022

Conversation

urandom2
Copy link
Contributor

@urandom2 urandom2 commented Nov 8, 2022

Description of changes

Fixes #186826

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Nov 8, 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/1380

@urandom2
Copy link
Contributor Author

urandom2 commented Nov 8, 2022

this also suffers from the .xxx-wrapped as arg0 bug, let me know what the canonical resolution to that is:

Tue 08 Nov 2022 12:42:17 UTC INFO - main - No arguments provided. See '.steamtinkerlaunch-wrapped --help' for possible command line parameters

@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/1415

@SuperSandro2000
Copy link
Member

this also suffers from the .xxx-wrapped as arg0 bug, let me know what the canonical resolution to that is:

Tue 08 Nov 2022 12:42:17 UTC INFO - main - No arguments provided. See '.steamtinkerlaunch-wrapped --help' for possible command line parameters

The error message nearly suggests that you did not provide any arguments but you can change argv0 with the argv0 argument. See https://github.com/NixOS/nixpkgs/blob/d291728eee55d5befa60421e84fb3e78e0ccc284/pkgs/build-support/setup-hooks/make-binary-wrapper/make-binary-wrapper.sh for details

@urandom2
Copy link
Contributor Author

I tried that, it appears to be the bash script introspecting its filename.

@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/1471

@urandom2 urandom2 force-pushed the steamtinkerlaunch branch 2 times, most recently from d17bdc6 to 4768861 Compare November 26, 2022 12:19
@SuperSandro2000
Copy link
Member

I tried that, it appears to be the bash script introspecting its filename.

Then we should rather patch that code.

@urandom2
Copy link
Contributor Author

urandom2 commented Nov 27, 2022 •
edited
Loading

ok, so this just looks like a bash behaviour, see upstream docs, you cannot lie about argv0; as such, I will patch the script to hardcode the binary name

EDIT: this is a documented bug in makeWrapper: #150841

@urandom2 urandom2 force-pushed the steamtinkerlaunch branch 2 times, most recently from 8cdc6c1 to c6ea776 Compare November 27, 2022 23:56
@urandom2 urandom2 mentioned this pull request Nov 28, 2022
Merged
13 tasks
@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/1483

SuperSandro2000
SuperSandro2000 reviewed Nov 29, 2022
View reviewed changes
pkgs/tools/games/steamtinkerlaunch/default.nix
hash = "sha256-rWENtgV6spokzkhnmrrzsAQ19dROJ50ofEulU5Jx5IE=";
};

# hardcode PROGCMD because #150841
Copy link
Member

Choose a reason for hiding this comment

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

Links should ideally be absolute

@SuperSandro2000 SuperSandro2000 merged commit 343ea40 into NixOS:master Nov 29, 2022
@urandom2 urandom2 deleted the steamtinkerlaunch branch December 1, 2022 14:55
@l0b0 l0b0 mentioned this pull request Jan 5, 2023
Merged
13 tasks
@urandom2 urandom2 mentioned this pull request Jan 10, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

Assignees
No one assigned
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Steam Tinker Launch
3 participants
@urandom2 @nixos-discourse @SuperSandro2000