-
Notifications
You must be signed in to change notification settings - Fork 1.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
Add Zig to ubuntu-rolling image, CI test #12953
Conversation
17a75fe
to
de99358
Compare
de99358
to
710ab8c
Compare
710ab8c
to
6f8b81e
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.
Status update as people have gotten re-interested in zig lately:
This PR fails the CI, which is a problem. You cannot add another workflow to os_comp.yml if it fails. Especially if we have no timeframe for when it might start working. ;)
@@ -58,6 +59,29 @@ source "$HOME/.cargo/env" | |||
rustup target add x86_64-pc-windows-gnu | |||
rustup target add arm-unknown-linux-gnueabihf | |||
|
|||
# Zig | |||
# Use the GitHub API to get the latest release information | |||
LATEST_RELEASE=$(wget -qO- "https://api.github.com/repos/ziglang/zig/releases/latest") |
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.
This is the wrong way to find the latest release and is broken. There's a JSON file which can be curl'd from the download page (https://ziglang.org/download/) and each entry has artifacts.
Edit: Thought it was broken because I misread a line down, this works but it still probably is a better idea to query the index.json
from the download page in case the download URL changes in the future.
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.
Not only isn't it broken, your reimplementation using the ziglang.org json file is broken.
I grow weary of nixos-brained people messing things up because they don't understand the basics of design.
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.
Not only isn't it broken, your reimplementation using the ziglang.org json file is broken.
CI failure is coming from the image builder failing which is happening upstream. I'm running a local test to ensure this works locally, from terminal testing it appears to work.
I grow weary of nixos-brained people messing things up because they don't understand the basics of design.
Um ouch
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.
Every distro seems to be cursed with a specific type of user that collects around it.
For Gentoo, of course, it is the people who insist on recompiling all software with -funroll-loops -fomg-optimize -Ofast.
For Arch, it is "btw I use arch" ricers who who only installed Arch in order to screenshot neofetch and are entirely unfamiliar with running any other aspect of Arch.
And for NixOS it is people who confidently report things are broken, cannot look at or produce logs, and wait for someone else to come point out that it isn't broken and something else is coincidentally broken at the same time but far away -- then ignore that and throw stuff at the wall until something sticks.
And frankly, I think it's pretty lame to tell someone their PR is broken because you "misread" it and didn't even try it, when the alternative changes you propose are actually detrimental to the design planning. In particular, your alternative inherently defeats the purpose of a CI for "project A" (meson) by instead running CI for "project B" (zig). Don't insult other contributors because you're sad that they didn't download random bizarroland nightly editions of a compiler instead of a stable release -- they had their reasoning for choosing stable releases and it's the same reason I share.
I'm sorry but this is just incredibly disappointing, and it is ALWAYS nixos users doing this stuff.
Whatever. I updated the PR myself to exclude setting os_comp.yml, in accordance with my original request that zig be installed in the CI beforehand. It also credits the original author.
Since you cannot update the CI image containers at the same time you start depending on them. Our CI is not currently architected in such a way as to allow updating the ci/ciimage/ definitions and have the os_comp.yml jobs use that in the same push -- you have to deliver the updated images to Docker Hub before CI will start running against them.
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.
Dude what? I was trying to offer constructive feedback. So what if I misread something, just point that out and leave it be. I didn't insult anyone. Also, don't group every single NixOS user together. I didn't bring up NixOS being anything related to this subject. Your comment feels really uncalled for and if this is how you see contributors, based on what distro they use or what projects they contribute to then I don't feel like it's worth contributing to this project. Meson has always been my favorite build tool as CMake is annoying to use and GNU Make/autotools is even more difficult. I'm just looking to make things better by contributing.
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.
@eli-schwartz That was… unnecessarily mean?
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.
Also, don't group every single NixOS user together. I didn't bring up NixOS being anything related to this subject. Your comment feels really uncalled for and if this is how you see contributors, based on what distro they use or what projects they contribute to
Not quite... I've used Arch for over a decade and then Gentoo for about a year, so if I actually just saw contributors based on what distro they use I'd have to be pretty judgmental of myself too.
Dude what? I was trying to offer constructive feedback.
Well, you didn't say what is wrong with it. Also I had already offered feedback saying that I had reviewed the PR and the parts in ci/ were good but the parts in .github/ "need to wait until the other PR is merged". So clearly, "entirely wrong" and "is broken" are, at best, statements that need to be clarified due to other people not seeing the issue you saw.
But ultimately, I have to say that it goes back to my comment above about "It also credits the original author."
Because if you take someone else's PR, tweak it slightly, and then submit it under your own name with you as the author, then... let's just say it predisposes me to view everything in the worst possible light. And while I could understand someone making a mistaken analysis in good faith and gently correct it, the story becomes a bit different when dealing with open source contribution laundering.
Please take this stuff seriously.
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.
Well, you didn't say what is wrong with it.
That is true, I did make that mistake. I thought this would've 404'd but I misread it.
But ultimately, I have to say that it goes back to my comment above about "It also credits the original author."
The changes I made are a lot simpler and different but both implementations are only a few lines. That PR also references this one. I was originally going to cherry-pick it but it was just easier to start that commit from scratch. My implementation also works on aarch64 and x86_64 so that's a plus.
Please take this stuff seriously.
I am, it really feels berating getting this kind of feedback. I already owned up to the mistake I made by editing the message and clarifying on Matrix. Getting this kind of feedback without asking "why is it broken" or how my reimplementation is broken as well doesn't seem constructive. All I've seen is walls of text with little to no substantial feedback.
@andy5995 ignore the nixos review please -- if you can update based on my review that will be fine... |
Signed-off-by: Eli Schwartz <[email protected]> [Eli: do not add to CI tests as this is only a preparatory PR]
They have recently upgraded to libgcrypt 1.11 and it has inherited the gpg suite migration to pkg-config.
6f8b81e
to
21eda4d
Compare
@RossComputerGuy I appreciate your effort and good intentions. Thank you. @eli-schwartz Thanks for fixing it up. |
#12293 (comment)
@eli-schwartz
This could be reviewed and merged here or I could do a PR against the branch and fork used in #12293 ?
/cc @Akaricchi