-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
rquickshare: build from source #368138
Conversation
9ba4afb
to
93c51c4
Compare
|
@ofborg build rquickshare-legacy |
93c51c4
to
e49f208
Compare
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 |
e49f208
to
6d03eda
Compare
There was a problem hiding this 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.
I blame Nvidia, but it might be useful for someone else |
6d03eda
to
199f8a2
Compare
Done! Thanks for the review Also rebased the branch to fix merge conflicts |
199f8a2
to
f7f3e75
Compare
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) |
This comment was marked as resolved.
This comment was marked as resolved.
Not just the folder path, there are differences in the dependencies and you even added a helper ( |
Legacy does change once in a while: https://github.com/Martichou/rquickshare/commits/master/app/legacy 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. |
I do not see how derivation is messy. I use 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 |
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. |
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 |
This comment was marked as outdated.
This comment was marked as outdated.
de927a2
to
185cc45
Compare
|
|
1 similar comment
|
185cc45
to
3e01957
Compare
|
|
|
It builds fine on my system
|
Getting a hash mismatch:
@PerchunPak can you invalidate the cache and make sure it is correct? |
Yes, everything still builds fine after running Nix GC. Also, |
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 |
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.
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 |
Try comparing the contents of your output of https://send.vis.ee/download/9c9d7cd87ab41ea7/#2OroC6exZcWIFJsWxvposg |
Ofborg is dead |
I will trust you, the worst case we can fix the hash later. |
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
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.