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

firefoxpwa: init at 2.7.3 #215905

Closed
wants to merge 2 commits into from
Closed

firefoxpwa: init at 2.7.3 #215905

wants to merge 2 commits into from

Conversation

pasqui23
Copy link
Contributor

@pasqui23 pasqui23 commented Feb 12, 2023

Description of changes

Fixes filips123/PWAsForFirefox#204
Fixes #186961

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/)
  • 23.05 Release Notes (or backporting 22.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.

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

@pasqui23 pasqui23 changed the title firefox-pwa: init at 2.1.2 firefox-pwa: init at 2.4.1 Feb 12, 2023
@pasqui23 pasqui23 changed the title firefox-pwa: init at 2.4.1 firefox-pwa: init at 2.4.1 [WIP] Feb 12, 2023
@pasqui23 pasqui23 changed the title firefox-pwa: init at 2.4.1 [WIP] firefox-pwa: init at 2.4.1 Feb 12, 2023
@pasqui23 pasqui23 force-pushed the firefox-pwa branch 2 times, most recently from 5bf1565 to 60f28e4 Compare February 12, 2023 16:38
@pasqui23 pasqui23 changed the title firefox-pwa: init at 2.4.1 firefoxpwa: init at 2.4.1 Feb 12, 2023
@@ -0,0 +1,90 @@
# <https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=firefox-pwa>
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
# <https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=firefox-pwa>

Comment on lines 38 to 40
# replace the version in the lockfile, otherwise Nix complains
sed -zi 's;name = "firefoxpwa"\nversion = "0.0.0";name = "firefoxpwa"\nversion = "${version}";' Cargo.lock
# replace the version number in the profile template files
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
# replace the version in the lockfile, otherwise Nix complains
sed -zi 's;name = "firefoxpwa"\nversion = "0.0.0";name = "firefoxpwa"\nversion = "${version}";' Cargo.lock
# replace the version number in the profile template files
sed -zi 's;name = "firefoxpwa"\nversion = "0.0.0";name = "firefoxpwa"\nversion = "${version}";' Cargo.lock

comments should explain the reasons behind a line, not what it is doing and the first comment is obvious.

Comment on lines 55 to 58
# Completions
install -Dm755 $target/completions/firefoxpwa.bash $out/share/bash-completion/completions/firefoxpwa
install -Dm755 $target/completions/firefoxpwa.fish $out/share/fish/vendor_completions.d/firefoxpwa.fish
install -Dm755 $target/completions/_firefoxpwa $out/share/zsh/vendor-completions/_firefoxpwa
Copy link
Member

Choose a reason for hiding this comment

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

Please use installCompletion

doCheck = false;

nativeBuildInputs = [ pkg-config installShellFiles ];
buildInputs = [ openssl.dev openssl ];
Copy link
Member

Choose a reason for hiding this comment

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

That looks fishy. Why do we need both?

nativeBuildInputs = [ pkg-config installShellFiles ];
buildInputs = [ openssl.dev openssl ];

preConfigure = ''
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 do this in postPatch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No or cargo complains

Copy link
Member

Choose a reason for hiding this comment

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

That means that our cargo.lock is out of date and needs to be fixed, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

buildRustPackage take the Cargo.lock from upstream, so the problem is there?

Choose a reason for hiding this comment

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

It's because I don't want to update the version in Cargo.toml every time before I release a new version. So, Cargo.toml in the repository only sets version = "0.0.0" and the actual version is then automatically set when building and releasing the package (in GitHub Actions for the official packages and here for Nix package).

Copy link
Member

Choose a reason for hiding this comment

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

We should patch that version to be correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aren't we already doing this?

Comment on lines 53 to 55
installShellCompletion --bash $target/completions/firefoxpwa.bash
installShellCompletion --fish $target/completions/firefoxpwa.fish
installShellCompletion --zsh $target/completions/_firefoxpwa
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
installShellCompletion --bash $target/completions/firefoxpwa.bash
installShellCompletion --fish $target/completions/firefoxpwa.fish
installShellCompletion --zsh $target/completions/_firefoxpwa
installShellCompletion --cmd firefoxpwa \
--bash $target/completions/firefoxpwa.bash \
--fish $target/completions/firefoxpwa.fish \
--zsh $target/completions/_firefoxpwa

@camillemndn
Copy link
Contributor

camillemndn commented Mar 12, 2023

Hi, so I managed to make the runtime-patched files writable via an upstream PR, I hope it will be accepted!

Now, it seems that the updater is broken, I guess the binary should be patched-elf to work with NixOS, but don't you think it could be interesting to wrap firefoxpwa with a script that softlinks the ${pkgs.firefox}/lib/firefox/* folder and disable auto-updates, to increase reproducibility and update via NixOS?

Also, the sound and notifications seem to be broken. Can you reproduce? Do you think firefoxpwa should be built with libpulseaudio and libnotify ?
I found a fix, inspired by the firefox derivation: camillemndn@9072e7f but I guess it could be improved.

Thanks!

@@ -25,9 +26,16 @@ let
pname = "${pname}-unwrapped";

src = "${source}/${dir}";
cargoSha256 = "sha256-G8CxvANwjh8rCuBtQbO0UseQTg40EL0eqW7GG/XTpOg=";
cargoLock = {
lockFile = "${unwrapped.src}/Cargo.lock";
Copy link
Member

Choose a reason for hiding this comment

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

I think this triggers IFD which is not allowed


nativeBuildInputs = [ pkg-config installShellFiles ];
nativeBuildInputs = [ pkg-config installShellFiles bintools ];
Copy link
Member

Choose a reason for hiding this comment

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

bintools should be part of stdenv

@pasqui23
Copy link
Contributor Author

pasqui23 commented Apr 19, 2023 via email

@camillemndn
Copy link
Contributor

So turns out the 2.5.0 version did not buils and I forgot to check. I tried to fix the build build but I got struck at yje following error:

; nix build .#firefoxpwa                                                                    # took 23m18s zsh 
warning: binary cache 'http://127.0.0.1:12304' is for Nix stores with prefix 'Nix::Store::getStoreDir', not '/nix/store'
error: builder for '/nix/store/2kxbhaqsxdvb8p98zvqmqrd9wnpv39dx-firefoxpwa-unwrapped-2.5.0.drv' failed with exit code 101;
       last 25 log lines:
       >   running: "gcc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-I" "bzip2-1.0.8" "-D_FILE_OFFSET_BITS=64" "-DBZ_NO_STDIO" "-o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/compress.o" "-c" "bzip2-1.0.8/compress.c"
       >   exit status: 0
       >   running: "gcc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-I" "bzip2-1.0.8" "-D_FILE_OFFSET_BITS=64" "-DBZ_NO_STDIO" "-o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/decompress.o" "-c" "bzip2-1.0.8/decompress.c"
       >   exit status: 0
       >   running: "gcc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-I" "bzip2-1.0.8" "-D_FILE_OFFSET_BITS=64" "-DBZ_NO_STDIO" "-o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/bzlib.o" "-c" "bzip2-1.0.8/bzlib.c"
       >   exit status: 0
       >   cargo:rerun-if-env-changed=AR_x86_64-unknown-linux-gnu
       >   AR_x86_64-unknown-linux-gnu = None
       >   cargo:rerun-if-env-changed=AR_x86_64_unknown_linux_gnu
       >   AR_x86_64_unknown_linux_gnu = None
       >   cargo:rerun-if-env-changed=HOST_AR
       >   HOST_AR = None
       >   cargo:rerun-if-env-changed=AR
       >   AR = Some("ar")
       >   running: "ar" "cq" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/libbz2.a" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/blocksort.o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/huffman.o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/crctable.o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/randtable.o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/compress.o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/decompress.o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/bzlib.o"
       >   exit status: 0
       >   running: "ar" "s" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/libbz2.a"
       >   exit status: 0
       >   cargo:rustc-link-lib=static=bz2
       >   cargo:rustc-link-search=native=/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib
       >
       >   --- stderr
       >   thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 13, kind: PermissionDenied, message: "Permission denied" }', /build/cargo-vendor-dir/bzip2-sys-0.1.11+1.0.8/build.rs:44:64
       >   note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
       > warning: build failed, waiting for other jobs to finish...
       For full logs, run 'nix log /nix/store/2kxbhaqsxdvb8p98zvqmqrd9wnpv39dx-firefoxpwa-unwrapped-2.5.0.drv'.
error: 1 dependencies of derivation '/nix/store/q84mn5n28bzngcvfai1lbywqbsrrhm4j-firefoxpwa-2.5.0.drv' failed to build

Hi, it builds for me on x86_64-linux, what is your architecture? It seems that you need to build bzip2 from source, but that I did not need to!

, fetchFromGitHub
, openssl
, symlinkJoin
, buildFHSUserEnv
Copy link
Contributor

Choose a reason for hiding this comment

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

I think buildFHSUserEnv has been renamed buildFHSEnv.

Also it seems like the FHS env script fails since the latest updates for me: the Firefox window does not open and there is no error. If I use the unwrapped binary, it works!

@pasqui23
Copy link
Contributor Author

So turns out the 2.5.0 version did not buils and I forgot to check. I tried to fix the build build but I got struck at yje following error:

; nix build .#firefoxpwa                                                                    # took 23m18s zsh 
warning: binary cache 'http://127.0.0.1:12304' is for Nix stores with prefix 'Nix::Store::getStoreDir', not '/nix/store'
error: builder for '/nix/store/2kxbhaqsxdvb8p98zvqmqrd9wnpv39dx-firefoxpwa-unwrapped-2.5.0.drv' failed with exit code 101;
       last 25 log lines:
       >   running: "gcc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-I" "bzip2-1.0.8" "-D_FILE_OFFSET_BITS=64" "-DBZ_NO_STDIO" "-o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/compress.o" "-c" "bzip2-1.0.8/compress.c"
       >   exit status: 0
       >   running: "gcc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-I" "bzip2-1.0.8" "-D_FILE_OFFSET_BITS=64" "-DBZ_NO_STDIO" "-o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/decompress.o" "-c" "bzip2-1.0.8/decompress.c"
       >   exit status: 0
       >   running: "gcc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-I" "bzip2-1.0.8" "-D_FILE_OFFSET_BITS=64" "-DBZ_NO_STDIO" "-o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/bzlib.o" "-c" "bzip2-1.0.8/bzlib.c"
       >   exit status: 0
       >   cargo:rerun-if-env-changed=AR_x86_64-unknown-linux-gnu
       >   AR_x86_64-unknown-linux-gnu = None
       >   cargo:rerun-if-env-changed=AR_x86_64_unknown_linux_gnu
       >   AR_x86_64_unknown_linux_gnu = None
       >   cargo:rerun-if-env-changed=HOST_AR
       >   HOST_AR = None
       >   cargo:rerun-if-env-changed=AR
       >   AR = Some("ar")
       >   running: "ar" "cq" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/libbz2.a" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/blocksort.o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/huffman.o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/crctable.o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/randtable.o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/compress.o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/decompress.o" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/bzip2-1.0.8/bzlib.o"
       >   exit status: 0
       >   running: "ar" "s" "/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib/libbz2.a"
       >   exit status: 0
       >   cargo:rustc-link-lib=static=bz2
       >   cargo:rustc-link-search=native=/build/native/target/x86_64-unknown-linux-gnu/release/build/bzip2-sys-3b02640410b495aa/out/lib
       >
       >   --- stderr
       >   thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 13, kind: PermissionDenied, message: "Permission denied" }', /build/cargo-vendor-dir/bzip2-sys-0.1.11+1.0.8/build.rs:44:64
       >   note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
       > warning: build failed, waiting for other jobs to finish...
       For full logs, run 'nix log /nix/store/2kxbhaqsxdvb8p98zvqmqrd9wnpv39dx-firefoxpwa-unwrapped-2.5.0.drv'.
error: 1 dependencies of derivation '/nix/store/q84mn5n28bzngcvfai1lbywqbsrrhm4j-firefoxpwa-2.5.0.drv' failed to build

Hi, it builds for me on x86_64-linux, what is your architecture? It seems that you need to build bzip2 from source, but that I did not need to!

The error is in the build of firefoxpwa-unwrapped,did you do any change to the code? Rebase to latest master?

@SuperSandro2000
Copy link
Member

Without bintools the ar command is not found

I think we want to use the ar in the AR env.

@pasqui23
Copy link
Contributor Author

pasqui23 commented Apr 30, 2023

I think we want to use the ar in the AR env.

After replacing bintools with ar I get

; nix build .#firefoxpwa                                                                       # zsh 
error: Function called without required argument "ar" at /nix/store/dsw0g2mmkjihhvjgk7jb9lvhjnph1wsl-source/pkgs/tools/networking/firefoxpwa/default.nix:9, did you mean "acr", "ag" or "air"?

@pwall2222

This comment was marked as outdated.

@pasqui23
Copy link
Contributor Author

Ok the problem was a missing bzip2 dependency. Added that and updated to 2.7.3

@pasqui23 pasqui23 changed the title firefoxpwa: init at 2.5.0 firefoxpwa: init at 2.7.3 Sep 16, 2023
@pasqui23 pasqui23 marked this pull request as ready for review September 16, 2023 17:42
@camillemndn camillemndn mentioned this pull request Oct 25, 2023
13 tasks
Co-authored-by: Camille M. <camillemondon@free.fr>
@camillemndn
Copy link
Contributor

Hi, Check out my PR, I made the update and created a test!

@camillemndn
Copy link
Contributor

More up to date here: #263404

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.

NixOS Packaging [Packaging Request] PWAsForFirefox
7 participants