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.

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

nix-universal-prefetch: init at 0.4.0 #326215

Closed
wants to merge 1 commit into from

Conversation

squalus
Copy link
Member

@squalus squalus commented Jul 10, 2024

Description of changes

This package was deleted in 578d7e7. This broke the Electron update script.

This fixes the Electron update script.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@squalus
Copy link
Member Author

squalus commented Jul 10, 2024

cc @yayayayaka and @teutat3s (Electron maintainers)

@Frontear
Copy link
Member

Frontear commented Jul 10, 2024

Does this have any issues with licensing, as afaik this was samueldr's script, whom since does not wish to participate in the community.

I just think it's potentially disrespectful to his wishes at best and potentially license violating at worst.

EDIT: Okay so its MIT so its not particularly license violating, but I do feel it might be a bit disrespectful to his wishes. Maybe the script should be reworked to not depend on it?

@squalus
Copy link
Member Author

squalus commented Jul 11, 2024

Does this have any issues with licensing, as afaik this was samueldr's script, whom since does not wish to participate in the community.

I just think it's potentially disrespectful to his wishes at best and potentially license violating at worst

No problem with licensing that I'm aware of, since it's an MIT license.

I cannot comment on the other issues. This is just a fix for some breakage that I found while trying to get a newer version of Electron working (v31).

@Frontear
Copy link
Member

I'd like some community feedback before this gets merged, hopefully some people stop by and comment their opinion.

@teutat3s
Copy link
Member

I'd like to try a solution like #322179 taking this as an opportunity.

This package was deleted in 578d7e7. This broke the Electron
update script.

This fixes the Electron update script.

Fixes NixOS#321490
@squalus squalus force-pushed the nix-universal-prefetch branch from 12dc1ff to 6757561 Compare July 11, 2024 07:50
@squalus
Copy link
Member Author

squalus commented Jul 11, 2024

Thanks for the link to the issue. I was unaware that there was a prior discussion. I also updated the PR to point to the version with intact git history.

I favor merging this PR so the update script can be fixed, since it's already been broken for some weeks.

If the update script is changed to use something else, I don't think there is a need for this PR or the package anymore.

@squalus
Copy link
Member Author

squalus commented Jul 11, 2024

Here's another option for fixing the script: #326364

@squalus squalus closed this Jul 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants