-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
tachyon: Add support for jpeg, png, more platforms #38119
Conversation
-#JPEGLIB= -L/usr/local/lib -ljpeg | ||
+USEJPEG= -DUSEJPEG | ||
+JPEGINC= -I/usr/include | ||
+JPEGLIB= -L/usr/lib -ljpeg |
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.
Did you try removing /usr/
mentions? I think we still try to remove things that break non-sandboxed non-NixOS builds if we already touch the corresponding lines.
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.
Very good point, changed.
It would probably be even easier to just re-write the Make-config
file entirely instead of patching it, but patching it has the advantage of breaking when upstream changes that file.
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.
I think -ljpeg
is still needed, though
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.
I replaced that by setting environment variables in preBuild
, which is easier to do conditionally and only requires a single patch.
21f91b3
to
a6e608a
Compare
The patch doesn't apply |
Thats weird, I just retried and it builds fine locally. Hunk #5 is
It would be nice to see that reject file, can you try to reproduce the failure? |
Nothing weird: a bump has happenned in the master branch. |
a6e608a
to
d250e33
Compare
.. and I just talked about the advantage of the patch breaking when the file changes 😁 I rebased on master. |
Success on x86_64-linux (full log) Attempted: tachyon Partial log (click to expand)
|
Ah okay I copied the new architectures from the sage spkg, but apparently that also needs a check to the makefile. I'll include that too. |
d250e33
to
327ba49
Compare
(Hopefully) fixed |
|
327ba49
to
dcf29c1
Compare
Sorry, I thought I tested that change but I must've forgot. Changed. |
@GrahamcOfBorg build tachyon |
Failure on x86_64-darwin (full log) Attempted: tachyon Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: tachyon Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: tachyon Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: tachyon Partial log (click to expand)
|
Failure on x86_64-darwin (full log) Attempted: tachyon Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: tachyon Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: tachyon Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: tachyon Partial log (click to expand)
|
Failure on x86_64-darwin (full log) Attempted: tachyon Partial log (click to expand)
|
There goes my attempt to clean up a bit. Anyway: apparently, Carbon did not get added correctly… |
Any idea why that might be? Otherwise I'll just disable the darwin build for now, at least arm works. |
Maybe it is plain |
dcf29c1
to
c7acd93
Compare
I removed the By the way, why is it
and not
? |
Because |
@GrahamcOfBorg eval @GrahamcOfBorg build tachyon |
Success on x86_64-linux (full log) Attempted: tachyon Partial log (click to expand)
|
No attempt on x86_64-darwin (full log) The following builds were skipped because they don't evaluate on x86_64-darwin: tachyon Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: tachyon Partial log (click to expand)
|
Ah makes sense. Thanks! |
Motivation for this change
Tachyon supports jpeg and png, but those need to be enabled at build time.
I also added some new platforms, I did not test on any platform besides 64 bit linux though. However, since it is supported upstream and I lifted the platform definitions from the (presumably well-tested) sage spkg, there is good reason to believe that they will work.
Pinging maintainer @7c6f434c.
Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)