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

rquickshare: build from source #368138

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

PerchunPak
Copy link
Member

The main version didn't work for me (it couldn't see my device or accept connections), so I decided to try the legacy one (it didn't work too). Anyway, I added rquickshare-legacy so users can easily access legacy build.

Closes #357017
Fixes #350315
Supersedes #357673
Supersedes #367605

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Dec 25, 2024
@PerchunPak PerchunPak force-pushed the rquickshare-from-source branch from 9ba4afb to 93c51c4 Compare December 25, 2024 15:16
@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Dec 25, 2024
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin labels Dec 25, 2024
@PerchunPak
Copy link
Member Author

PerchunPak commented Dec 25, 2024

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 368138


x86_64-linux

✅ 2 packages built:
  • rquickshare
  • rquickshare-legacy

aarch64-linux

❌ 2 packages failed to build:
  • rquickshare
  • rquickshare-legacy

EDIT: Looks like virtualization problem, can someone with actual aarch hardware confirm these build failures?

    Error failed to build app:
    - Target aarch64-unknown-linux-gnu does not exist. Please run `rustup target list` to see the available targets.
$ rustup target list | rg aarch64-unknown-linux-gnu
aarch64-unknown-linux-gnu

EDIT2: ofborg builds this successfully

@PerchunPak
Copy link
Member Author

@ofborg build rquickshare-legacy

@PerchunPak PerchunPak force-pushed the rquickshare-from-source branch from 93c51c4 to e49f208 Compare January 2, 2025 08:54
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin and removed 10.rebuild-darwin: 1-10 labels Jan 2, 2025
@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/5037

@PerchunPak PerchunPak force-pushed the rquickshare-from-source branch from e49f208 to 6d03eda Compare January 3, 2025 11:12
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
pkgs/by-name/rq/rquickshare/package.nix Outdated Show resolved Hide resolved
Copy link
Contributor

@thiagokokada thiagokokada left a comment

Choose a reason for hiding this comment

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

BTW, I think this derivation would make more sense if it was 2 separate files instead, one for rquickshare and another for rquickshare-legacy. Having everything in one makes the things more complicated for no reason.

so I decided to try the legacy one (it didn't work too)

If legacy is not working please just remove it.

@PerchunPak
Copy link
Member Author

If legacy is not working please just remove it.

I blame Nvidia, but it might be useful for someone else

@PerchunPak PerchunPak force-pushed the rquickshare-from-source branch from 6d03eda to 199f8a2 Compare January 7, 2025 17:04
@PerchunPak
Copy link
Member Author

PerchunPak commented Jan 7, 2025

Done! Thanks for the review

Also rebased the branch to fix merge conflicts

@PerchunPak PerchunPak force-pushed the rquickshare-from-source branch from 199f8a2 to f7f3e75 Compare January 7, 2025 17:06
@PerchunPak
Copy link
Member Author

BTW, I think this derivation would make more sense if it was 2 separate files instead

Why? The difference is just the folder path. Upstream tries to keep them similar

The difference between them is just the tauri version (v1 vs v2)

@PerchunPak

This comment was marked as resolved.

@thiagokokada
Copy link
Contributor

BTW, I think this derivation would make more sense if it was 2 separate files instead

Why? The difference is just the folder path. Upstream tries to keep them similar

The difference between them is just the tauri version (v1 vs v2)

Not just the folder path, there are differences in the dependencies and you even added a helper (app-type-either) to smooth out the differences. But this just made the whole derivation messy, and considering that legacy will not change in the future I think just splitting the derivations would be easier.

@PerchunPak
Copy link
Member Author

PerchunPak commented Jan 7, 2025

legacy will not change in the future

Legacy does change once in a while:

https://github.com/Martichou/rquickshare/commits/master/app/legacy
https://github.com/Martichou/rquickshare/commits/master/app/main

Upstream tries to keep them in sync in terms of functionality, because v2 tauri introduces regressions for someone and at the same time fixes bugs for others.

@thiagokokada
Copy link
Contributor

thiagokokada commented Jan 7, 2025

Upstream tries to keep them in sync in terms of functionality, because v2 tauri introduces regressions for someone and at the same time fixes bugs for others.

One more reason to separate the derivations then.

@PerchunPak
Copy link
Member Author

I do not see how derivation is messy. I use app-type-either only for hashes, tauri & dependencies versions and meta.mainProgram.

Splitting the build would mean just a duplication of code and doubling maintenance required (every change that is done for the main version, would be need to be done for the legacy one). In case if mainstream would stop updating their legacy version, we should consider just deprecating it and later removing it. Right now, the philosophy of upstream is to keep them identical to user

I provide the legacy version only because upstream also provides binaries for it in their releases
image
https://github.com/Martichou/rquickshare/releases/tag/v0.11.2

@thiagokokada
Copy link
Contributor

Splitting the build would mean just a duplication of code and doubling maintenance required (every change that is done for the main version, would be need to be done for the legacy one).

The idea is exactly avoid this, because if there is a change that only affects one version you would need to only change that version.

But anyway, fine. I am against of merging in this state but another committer can do so.

@PerchunPak
Copy link
Member Author

if there is a change that only affects one version you would need to only change that version.

The problem is that upstream has never showed any evidence that this will ever happen, so right now this would be just duplicating work. If it happens, it is easy enough to just fork the derivation

@PerchunPak

This comment was marked as outdated.

@PerchunPak PerchunPak force-pushed the rquickshare-from-source branch 2 times, most recently from de927a2 to 185cc45 Compare January 8, 2025 11:02
@thiagokokada
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 368138


x86_64-linux

❌ 2 packages failed to build:
  • rquickshare
  • rquickshare-legacy

@thiagokokada
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 368138

1 similar comment
@thiagokokada
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 368138

@PerchunPak PerchunPak force-pushed the rquickshare-from-source branch from 185cc45 to 3e01957 Compare January 8, 2025 12:11
@thiagokokada
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 368138

@thiagokokada
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 368138


x86_64-linux

❌ 2 packages failed to build:
  • rquickshare
  • rquickshare-legacy

@thiagokokada
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 368138

@PerchunPak
Copy link
Member Author

It builds fine on my system

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 368138


x86_64-linux

✅ 2 packages built:
  • rquickshare
  • rquickshare-legacy

aarch64-linux

✅ 2 packages built:
  • rquickshare
  • rquickshare-legacy

@thiagokokada
Copy link
Contributor

Getting a hash mismatch:

 error: hash mismatch in fixed-output derivation '/nix/store/cwl6z71k7qyks4c6bn0hjvhmlx7vmfwy-rquickshare-pnpm-deps.drv':
         specified: sha256-V46V/VPwCKEe3sAp8zK0UUU5YigqgYh1GIOorqIAiNE=
            got:    sha256-gEQ5odnvYac6A0oY3l9f4CDwMrOjfqYWiARJk3NUuSM=
error: hash mismatch in fixed-output derivation '/nix/store/n265f6fb23xsl0asd8k1q7igl9g9yj9m-rquickshare-legacy-pnpm-deps.drv':
         specified: sha256-sDHysaKMdNcbL1szww7/wg0bGHOnEKsKoySZJJCcPik=
            got:    sha256-qLIIQ3OeKFLg0bbWmrZyvHdjYku5OujCJuTxmPCcHt4=

@PerchunPak can you invalidate the cache and make sure it is correct?

@PerchunPak
Copy link
Member Author

PerchunPak commented Jan 8, 2025

Yes, everything still builds fine after running Nix GC. Also, nom build .#rquickshare.pnpmDeps --rebuild succeeds on aarch64-build-box.nix-community.org

@PerchunPak
Copy link
Member Author

PerchunPak commented Jan 8, 2025

What version of Nix do you use? Maybe you are on an old version, because your nixpkgs-review doesn't output one comment for all systems (that feature was added only half a year ago)

Mine is nix (Lix, like Nix) 2.92.0-dev-pre20250105-8c1ece9, aarch64-build-box.nix-community.org: nix (Nix) 2.25.3

@thiagokokada
Copy link
Contributor

thiagokokada commented Jan 8, 2025

What version of Nix do you use? Maybe you are on an old version, because your nixpkgs-review doesn't output one comment for all systems (that feature was added only half a year ago)

The reason it is doing one output for each system is because I am building in separate systems.

Nix version is not relevant anyway since indenpendent of the version we need to generate the same hash.

Yes, everything still builds fine after running Nix GC. Also, nom build .#rquickshare.pnpmDeps --rebuild succeeds on aarch64-build-box.nix-community.org

Let's wait to see if ofborg builds correctly then. Wouldn't be the first time that I see those kind of hash issues. I think pnpm fetcher is introducing some impurities.

@PerchunPak
Copy link
Member Author

Try comparing the contents of your output of rquickshare.pnpmDeps with mine

https://send.vis.ee/download/9c9d7cd87ab41ea7/#2OroC6exZcWIFJsWxvposg

@PerchunPak
Copy link
Member Author

Let's wait to see if ofborg builds correctly then. Wouldn't be the first time that I see those kind of hash issues. I think pnpm fetcher is introducing some impurities.

Ofborg is dead
https://discourse.nixos.org/t/infrastructure-announcement-the-future-of-ofborg-your-help-needed/56025

@thiagokokada
Copy link
Contributor

thiagokokada commented Jan 8, 2025

Try comparing the contents of your output of rquickshare.pnpmDeps with mine

https://send.vis.ee/download/9c9d7cd87ab41ea7/#2OroC6exZcWIFJsWxvposg

I will trust you, the worst case we can fix the hash later.

@thiagokokada thiagokokada merged commit 1b85ede into NixOS:master Jan 8, 2025
23 of 27 checks passed
@gepbird gepbird mentioned this pull request Jan 12, 2025
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update request: rquickshare build from source rquickshare: "Cannot create EGL context: invalid display"
4 participants