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

update etcher version - will update to electron v25 #274476

Closed
wants to merge 6 commits into from
Closed

update etcher version - will update to electron v25 #274476

wants to merge 6 commits into from

Conversation

andar1an
Copy link

@andar1an andar1an commented Dec 15, 2023

Description of changes

balena-io/etcher@f38bca2 (patch: upgrade to electron 25, 2023-08-30)
balena-io/etcher@fb8ed5b (patch: refactor scanner, loader and flasher out of gui + upgrade to electron 25, 2023-08-30)

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.05 Release Notes (or backporting 23.05 and 23.11 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.

👍 - Security related update
cc @wegank

@andar1an
Copy link
Author

This is my first pull request for a pkg update, please let me know if I did something wrong, or did not do something I was supposed to do.

@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Dec 15, 2023
@wegank
Copy link
Member

wegank commented Dec 15, 2023

There is an unresolved problem upstream (balena-io/etcher#4138) that blocks all previous update attempts at #261626 and #263729. Have you actually tested flashing an image onto a USB stick using your build?

@andar1an
Copy link
Author

No, I will look up how to build locally to test.

@andar1an
Copy link
Author

andar1an commented Dec 15, 2023

Do you happen to know of any blog or easily digestible content related to building and testing packages for nikpgs?

I found this in the docs https://github.com/jtojnar/nixpkgs-hammering, there is a lot to read though, and something more concise would be helpful if you know of anything.

Update: this seems like viable candidate https://elatov.github.io/2022/01/building-a-nix-package/, leads to https://nixos.wiki/wiki/Nixpkgs/Contributing

@wegank
Copy link
Member

wegank commented Dec 15, 2023

For contributions to nixpkgs, you can find some instructions in pkgs/README.md. Specifically, for building etcher, you'll need to run this from the root of the nixpkgs source tree:

$ NIXPKGS_ALLOW_INSECURE=1 nix-build -A etcher

NIXPKGS_ALLOW_INSECURE=1 is something that will be prompted when running nix-build -A etcher, since both Electron 19 and 25 are marked as insecure in nixpkgs.

@ofborg ofborg bot requested a review from wegank December 15, 2023 15:23
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Dec 15, 2023
@andar1an
Copy link
Author

andar1an commented Dec 15, 2023

For contributions to nixpkgs, you can find some instructions in pkgs/README.md. Specifically, for building etcher, you'll need to run this from the root of the nixpkgs source tree:

$ NIXPKGS_ALLOW_INSECURE=1 nix-build -A etcher

NIXPKGS_ALLOW_INSECURE=1 is something that will be prompted when running nix-build -A etcher, since both Electron 19 and 25 are marked as insecure in nixpkgs.

Oh no, I was hoping going to 25 would remove that security prompt. Thank you for this, I am doing now. I now see latest electron is v28.

@andar1an
Copy link
Author

It builds, I will test package now with sudo nixos-rebuild switch -I nixpkgs=/path/to/local/nixpkgs and see what happens.

@andar1an
Copy link
Author

andar1an commented Dec 15, 2023

Seemed to flash for me, and validated. I did receive a partition table warning, but I imagine that is because of the image I had on this USB already. Let me verify versions of everything though.
image

@andar1an
Copy link
Author

andar1an commented Dec 15, 2023

Seems when I comment out the permittedInsecurePackages config, that it is still referencing electron-19.1.9 when the subsequent error occurs.

I ran this command: sudo nixos-rebuild switch -I nixpkgs=/home/stephen/Repos/github/nixpkgs --flake /home/stephen/Repos/gitlab/flakes/machines#tor-dev-01

I imagine that a flake inputs override the -I flag, so I can try updating input to a local path.

Update: I think I need to update the electron dependency version somehow in the PR. Local path works in flake, but still reverences v19.1.9. Looking into now, but I don't see how the dependency version is determined yet in etcher default.nix. I do see electron passed in as input though, so will trace.

@wegank
Copy link
Member

wegank commented Dec 15, 2023

Update: I think I need to update the electron dependency version somehow in the PR. Local path works in flake, but still reverences v19.1.9. Looking into now, but I don't see how the dependency version is determined yet in etcher default.nix. I do see electron passed in as input though, so will trace.

For doing this, you also have to change electron_19 to electron_25 somewhere in the pkgs/top-level/all-packages.nix.

But what's interesting is that you've discovered that Electron 1.18.13 works with Electron 19. I'll check whether this is the case on my machine, and if it is, we can definitely do the upgrade.

@andar1an
Copy link
Author

Update: I think I need to update the electron dependency version somehow in the PR. Local path works in flake, but still reverences v19.1.9. Looking into now, but I don't see how the dependency version is determined yet in etcher default.nix. I do see electron passed in as input though, so will trace.

For doing this, you also have to change electron_19 to electron_25 somewhere in the pkgs/top-level/all-packages.nix.

But what's interesting is that you've discovered that Electron 1.18.13 works with Electron 19. I'll check whether this is the case on my machine, and if it is, we can definitely do the upgrade.

I need to learn about all-packages.nix now, but was hoping this upgrade would move etcher to electron 25 also.

@wegank
Copy link
Member

wegank commented Dec 15, 2023

But what's interesting is that you've discovered that Electron 1.18.13 works with Electron 19. I'll check whether this is the case on my machine, and if it is, we can definitely do the upgrade.

This is not the case on my machine, unfortunately... Does Etcher report itself as 1.18.13 when clicked on the settings button?

@andar1an
Copy link
Author

But what's interesting is that you've discovered that Electron 1.18.13 works with Electron 19. I'll check whether this is the case on my machine, and if it is, we can definitely do the upgrade.

This is not the case on my machine, unfortunately... Does Etcher report itself as 1.18.13 when clicked on the settings button?

No, 1.18.2. I will restart machine and see if that changes anything.

@andar1an
Copy link
Author

andar1an commented Dec 15, 2023

Builds have been taking a long time since removing etcher, clearing garbage, and restarting. I am going to put a pin in this for today.

@wegank wegank marked this pull request as draft December 15, 2023 22:25
@andar1an
Copy link
Author

I won't have time to finish this so closing.

@andar1an andar1an closed this Jan 12, 2024
@andar1an andar1an deleted the etcher_1.18.3_update branch January 12, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 10.rebuild-linux: 1 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants