sony-headphones-client: fix build for aarch64 #201971
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
maybe we should instead set it to 255? and not remove it completely? |
|
Ideally we'd report our issues upstream and have them decide the proper course of action. |
mweinelt
reviewed
Nov 19, 2022
Stunkymonkey
force-pushed
the
sony-headphones-client-fix-aarch
branch
from
6fcef76
to
069b70f
Compare
|
Nope. |
ofborg
bot
added
11.by: package-maintainer
|
Builds, but please report these issues upstream. |
|
is already done in Plutoberth/SonyHeadphonesClient#82 |
|
I expect the fix to be: Plutoberth/SonyHeadphonesClient#84 |
Stunkymonkey
force-pushed
the
sony-headphones-client-fix-aarch
branch
2 times, most recently
from
f9a1ca4
to
7f4a3bc
Compare
|
@mweinelt could you please test it with aarch again? |
|
ping @mweinelt |
mweinelt
reviewed
Nov 28, 2022
Comment on lines
10
to
13
| rev = "v${version}"; | ||
| hash = "sha256-0DQanrglJiGsN8qQ5KxkL8I+Fpt1abeeuKiM8v9GclM="; | ||
| #rev = "v${version}"; | ||
| rev = "c8f60c030bfe1987415d05241a6839648d40b358"; | ||
| hash = "sha256-xRbPFaPlO5hyDQJyM4Pf56icRcJAL9H46Ep+hcl3DUg="; | ||
| fetchSubmodules = true; |
There was a problem hiding this comment.
Can we fetch this as a patch instead of touching that logic?
There was a problem hiding this comment.
I feel this is the least effort solution. Otherwise I will have to cleanup later...
(Additionally the original patch from github can not be reuesed due to the different sourceRoot)
There was a problem hiding this comment.
Applying a patch onto a src that uses a tag is a much more semantically valuable approach.
There was a problem hiding this comment.
seems more work, but sure. For anybody else this is easier to understand.
There was a problem hiding this comment.
@mweinelt please have a look if this is the way you wanted it.
Stunkymonkey
force-pushed
the
sony-headphones-client-fix-aarch
branch
from
7f4a3bc
to
424b63a
Compare
mweinelt
reviewed
Dec 1, 2022
| --replace "UNKNOWN = -1" "// UNKNOWN removed since it doesn't fit in char" | ||
| ''; | ||
| patches = [ | ||
| ./fix-aarch64.patch |
There was a problem hiding this comment.
Huh, can't we fetchpatch the patch? Is this not a patch you submitted upstream?
There was a problem hiding this comment.
yes it is.
The problem is, that the patch itself is referring to the file Client/Constants.h.
Due to the changed sourceRoot into the Client folder, the path does not match... Therefore I had to adjust the path manually.
Fetching the commit while pulling the source needs the least effort, the least changes, but is unusual for other contributors.
That was what i proposed before.
There was a problem hiding this comment.
Can you try to change into the sourceRoot in postPatch?
There was a problem hiding this comment.
that is easier said than done. I do not know how to easily fix this.
There was a problem hiding this comment.
please consider this provided change as a fix. All other solutions will have much more changes.
There was a problem hiding this comment.
So I guess we will wait for the next release. Fixing the build seems not important...
Stunkymonkey
force-pushed
the
sony-headphones-client-fix-aarch
branch
from
424b63a
to
7fa49a7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes
followup of #201231.
related to #199919
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes