python311Packages.babeltrace: init at 1.5.11 #310617
python311Packages.babeltrace: init at 1.5.11 #310617doronbehar merged 7 commits intoNixOS:masterfrom
Conversation
|
Result of 7 packages marked as broken and skipped:
3 packages failed to build:
57 packages built:
|
|
|
doronbehar
left a comment
There was a problem hiding this comment.
P.S If you'd use python311Packages.babeltrace and not python3Packages.babeltrace in the commit prefixes, you'd convince ofborg to build this attributes for you automatically. (The python3Packages prefixed attributes are not discovered by Hydra and hence are not discoved by ofborg as well).
There was a problem hiding this comment.
And then you'd be able to use pname and version without the finalAttrs. prefix...
There was a problem hiding this comment.
I removed finalAttrs completely, because it's not used for anything. I've also removed the second level of function calls so that there is just one attrset of parameters.
|
Sorry it took so long to discover this PR.. It looks good overall. |
13f0bad to
ff1d41e
Compare
|
So I addressed most of the @doronbehar's comments including squashing commits together and updating commit titles to trigger ofborg. Now, I'm running |
|
Result of 16 packages marked as broken and skipped:
60 packages built:
|
|
I'm also not sure that the right thing to do here is to separate the expressions for the v1 and version 2 attributes... If they are separate, they might eventually change through out time differently. If the current |
ff1d41e to
16ccb10
Compare
|
I rebased to nixos-unstable and separated v1 and v2 to their own independent packages. The result is really simpler. @bjornfor I added myself as co-maintainer to v1. Do you agree? Other changes:
|
16ccb10 to
023ae8b
Compare
|
Now, even v1 uses @hacker1024 It seems that cross-compilation to aarch64 was important for you. Cross-compilation works with Python 3.11, but fails with 3.12. Is it ok for you? |
Yes. Thanks! |
|
Result of 11 packages marked as broken and skipped:
60 packages built:
|
That's fine for me for now, I'm happy to look into it myself for 3.12 later. I'll probably need it once we switch to ROS Jazzy Jalisco with Python 3.12. |
There was a problem hiding this comment.
Do you think we can unconditionally enable man pages and use outputs = [ "out" "man" "dev" ]; so they won't take too much space in closure if someone doesn't want them?
There was a problem hiding this comment.
Just out of curiosity, is upstream aware that they need these loosening CFLAGS to make it work? It'd be nice to put a link if they do.
There was a problem hiding this comment.
I don't see any upstream commit addressing that. I'll let them know later, I'm in hurry right know. I just put there a comment that its needed when cross-compiling Python bindings.
There was a problem hiding this comment.
After the 1.5.8 -> 1.5.11 commit, or even before it, it'd be nice to format with nixfmt the expression, and also switch to finalAttrs: pattern.
There was a problem hiding this comment.
I reformatted it as last commit (no time for resolving conflicts right now :-).
Also, is there any advantage of adding the finalAttrs parameter if it is not used in the actual Nix expression? I don't think so and I believe that it makes evaluation a bit slower.
There was a problem hiding this comment.
Here we could replace rec and version with finalAttrs, but it doesn't help much, because we would need to override the whole src due to hash change. Or am I missing something?
Could you share please how it fails? Maybe I can help. |
|
Result of 16 packages marked as broken and skipped:
13 packages failed to build:
47 packages built:
|
It fails when cross-compiling Python itself: |
Could you share in a gist the full build log please? |
|
Full python build log as produced by |
Probably unrelated. Now I see the same, yesterday it was OK. |
cc @FRidh . |
Yes it could be failures related to the processing power of the machine you used. @siraben try to use |
|
@doronbehar Ok, rerunning. |
023ae8b to
50dd7c2
Compare
|
Addressed most of @doronbehar's comments. |
- Don't use `meta = with lib;`.
- Use (finalAttrs:{}) pattern.
- Use lib.enableFeature for debug-info ./configure flag.
- Use pkgs/by-name
50dd7c2 to
2f6b127
Compare
Indeed the rebase wasn't trivial :) Now it is done.
Yes. Using
So I hadn't had the patience to wait for it to be perfect, so I did it all myself. Your final approval on my changes will be great, if not, I'll simply wait for CI. |
2f6b127 to
e8ca973
Compare
e8ca973 to
2536c9c
Compare
Separate from babeltrace 1.x as "the Babeltrace 2 project is completely independent from Babeltrace 1". https://github.com/efficios/babeltrace/releases/tag/v2.0.0 Co-authored-by: hacker1024 <[email protected]>
A new attribute overriding babeltrace2 is created, rather than enabling Python support in babeltrace2 itself, in order to reduce the closure size and leave babeltrace2 Python-version-agnostic. Co-authored-by: hacker1024 <[email protected]>
2536c9c to
68bc018
Compare
Great. I was not aware of that.
Thank you for finishing and merging this. I was traveling with my family - sorry for not having time to give it a final testing. |
Description of changes
Rebase of #285681 plus additional fixes.
In the original PR, @hacker1024 (who agrees with this PR) says:
My additions to that are:
nixpkgs-reviewCloses #285681
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-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.