Skip to content
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

Release pipeline #2620

Merged
merged 11 commits into from
Jul 2, 2023
Merged

Release pipeline #2620

merged 11 commits into from
Jul 2, 2023

Conversation

owenthereal
Copy link
Member

@owenthereal owenthereal commented Jun 18, 2023

Add the release pipeline for jq. For now, it uploads build artifacts to a draft GH release. In the future, it can trigger an update of the website when a release is cut, for example.

This PR includes the following changes:

  1. Group CI builds for different OSes into ci.yml.
  2. Add release job to release jq when a tag is created in the format of v*.
  3. Try to use clang as the compiler on CI for Linux & MacOS. I couldn't get CC=clang to work for Windows, though (test failures) clang compiler doesn't work for msys2: MSYS2 clang va_start is broken msys2/MSYS2-packages#2291. We have to fall back to gcc for Windows build.
  4. Provide an extensible matrix for future cross-compile builds, e.g. for Build macOS arm64 variants #2618.

An example release can be checked out here.

@reneleonhardt
Copy link

4. Provide an extensible matrix for future cross-compile builds, e.g. for [Build macOS arm64 variants #2618](https://github.com/jqlang/jq/pull/2618).

Cool, I had the same idea, but first the new arm64 builds have to be accepted ;)

An example release can be checked out here.

Do you still have the binaries locally? They seem not to available anymore:
https://github.com/owenthereal/jq/actions/runs/5302079296/jobs/9596773009

@owenthereal
Copy link
Member Author

Do you still have the binaries locally? They seem not to available anymore:

The uploaded artifacts are on the "Summary" page, e.g. the one that failed the Windows build: https://github.com/owenthereal/jq/actions/runs/5302038760. But the Windows binary is not there because it didn't get to the end of a successful build.

@itchyny itchyny added this to the 1.7 release milestone Jun 25, 2023
* Group CI builds for different OSes into `ci.yml`.
* Add release job to release `jq` when tag is in the format of v*.
* Use `clang` as the only compiler on CI.
* Provide extensible matrix for future cross-compile builds, e.g.
  for jqlang#2618.
`brew update` can fail for "Error: Fetching /usr/local/Homebrew/Library/Taps/homebrew/homebrew-cask failed!"

Ref: https://github.com/owenthereal/jq/actions/runs/5432314910/jobs/9879266028#step:3:19
@owenthereal owenthereal force-pushed the owenthereal/release branch from 2a5fd27 to d45ae41 Compare July 1, 2023 18:28
@owenthereal
Copy link
Member Author

owenthereal commented Jul 1, 2023

Thank you @wader for helping with debugging the clang build issue on Windows ❤️ ! For those who are interested, MSYS2 doesn't officially support clang target: msys2/MSYS2-packages#2291 that's why our Windows tests were failing. For Windows build, we would have to fallback to the gcc target.

I updated this PR to build for both gcc & clang for Linux & MacOS while only testing gcc for Windows. I also expanded make check for all tests on Windows (they all pass!).

To release binaries, gcc builds are selected for all platforms. An example release can be found here.

This PR is ready to review again.

cc: @itchyny @wader

@owenthereal owenthereal requested review from itchyny and a team and removed request for itchyny July 1, 2023 18:41
@owenthereal owenthereal force-pushed the owenthereal/release branch from d45ae41 to 9ca8227 Compare July 1, 2023 18:46
@owenthereal owenthereal force-pushed the owenthereal/release branch from 9ca8227 to 6cce62d Compare July 1, 2023 18:46
@wader
Copy link
Member

wader commented Jul 1, 2023

Does anyone know how compatible the ubuntu binaries will be? will they run on most modern linux distributions? thinking glibc compatibility etc. If it's a compatibility mess would it make sense to build them static somehow?

@wader
Copy link
Member

wader commented Jul 1, 2023

Is the plan to add arm64 in this PR or do that separately?

@owenthereal
Copy link
Member Author

owenthereal commented Jul 1, 2023

Is the plan to add arm64 in this PR or do that separately?

I'm planning to do that separately. I haven't had a chance to verify an arm64 build on the amd64 CI will work on an arm64 machine.

@wader
Copy link
Member

wader commented Jul 1, 2023

The example release link seems to return 404... also maybe i should invest in a windows VM so i can actually test things properly :)

@owenthereal owenthereal force-pushed the owenthereal/release branch from 487a15a to 8dffb23 Compare July 1, 2023 19:13
@@ -15,7 +15,7 @@ jobs:
image: [ubuntu-20.04, ubuntu-22.04]
runs-on: ${{ matrix.image }}
env:
CC: ${{ matrix.compiler }}
CC: ${{ matrix.compiler }} -static
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should be able to do this via configure somehow, there is --enable-all-static which seems to use ld flag -all-static which i don't think i've used myself before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glibc can be a pain to build static with but jq i hope can be fine as i think the most pain comes from programs that need NSS or DNS

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use the --enable-static & --enable-all-static flags for LInux & MacOS, Windows already does that: 2480dd2

Copy link
Member Author

@owenthereal owenthereal Jul 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm...tests failed for ubuntu 20.04 after static is enabled (22.04 was passing though) : https://github.com/jqlang/jq/actions/runs/5432682781. They failed for "Segmentation fault", e.g. https://github.com/jqlang/jq/suites/14001000347/artifacts/781225809

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not good :( maybe revert for now? i can digg into it later if you want

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please help :). I disabled static build for ubuntu 20.04 for now: 6520242. Feel free to enable it once it's fixed.

@owenthereal
Copy link
Member Author

The example release link seems to return 404... also maybe i should invest in a windows VM so i can actually test things properly :)

I updated the example release link. Sorry for the inconvenience.

@owenthereal owenthereal force-pushed the owenthereal/release branch from 8dffb23 to 2480dd2 Compare July 1, 2023 19:18
libtool \
flex \
bison
sed -i.bak '/^AM_INIT_AUTOMAKE(\[-Wno-portability 1\.14\])$/s/14/11/' modules/oniguruma/configure.ac
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was this thing about?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line was inherited from the old CI setup. I believe it was to enforce onigurma's automake to be on the same version as jq. I confirmed this is not needed now. Removing.

submodules: true
- name: Install packages
run: |
# brew update sometimes fails with "Fetching /usr/local/Homebrew/Library/Taps/homebrew/homebrew-cask failed!"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this on some CI builds but for some reason (randomly?) only on macOS 12

Copy link
Member Author

@owenthereal owenthereal Jul 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that's what I saw. Perhaps the GH Actions macos-12 image has some issue? The update-reset fallback helps with that, as you see.

configure_flag: --enable-static --enable-all-static
- compiler: clang
image: ubuntu-20.04
configure_flag: ''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh so only ubuntu-20.04 and clang fails using static? other ubuntus are fine? 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah no also gcc, misread above

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can reproduce with ubuntu docker image:

$ docker run -ti -v $PWD:$PWD -w $PWD ubuntu:20.04
... <configure, build etc>...
root@83a347734b03:/Users/wader/src/jq# gdb --args ./jq
GNU gdb (Ubuntu 9.2-0ubuntu1~20.04.1) 9.2
....
(gdb) r
Starting program: /Users/wader/src/jq/jq
warning: Error disabling address space randomization: Operation not permitted

Program received signal SIGSEGV, Segmentation fault.
0x0000000000000000 in ?? ()
(gdb) bt
#0  0x0000000000000000 in ?? ()
#1  0x00000000004ec78c in __register_frame_info_bases.part.0 ()
#2  0x0000000000401ec1 in frame_dummy ()
#3  0x0000000000000001 in ?? ()
#4  0x00000000004edd3c in __libc_csu_init ()
#5  0x00000000004ed48e in __libc_start_main ()
#6  0x0000000000401dde in _start ()
(gdb)

Dugg a bit, i suspect it might be something like https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95989 some version of glibc pthreads don't like static linking. Usually you can make it work by providing a lots of link flags to tell it to resolve weak symbols and whatnot, but it's a mess.

Also got me thinking: do the jq cli need threads? looking at the code it seems to be about making libjq possible to use concurrently? but it's probably no easy task to make it optional

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also see that jq do call some libc functions like getpwuid that i suspect might trigger NSS usage that i think might trigger dynamic loading and might crash hmm

In my experience if one really want static binaries linking with musl is the "safest"... but that might have some other issues.... but again in my experience with modern musl versions there are few issues

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw alpine, that uses musl, already package jq and i haven't had any issues with it and looking at their bug tracker for jq issues https://gitlab.alpinelinux.org/alpine/aports/-/issues/?search=jq&sort=created_date&state=closed&first_page_size=20 i don't see anything that looks like reported random strange crashes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a test using CC=musl-gcc (needs package musl musl-dev musl-tools) and it seems to work fine and make check passes

root@83a347734b03:/Users/wader/src/jq# file jq
jq: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), statically linked, with debug_info, not stripped
root@83a347734b03:/Users/wader/src/jq# ls -lh jq
-rwxr-xr-x 1 root root 1.8M Jul  2 14:04 jq
root@83a347734b03:/Users/wader/src/jq# strip jq
root@83a347734b03:/Users/wader/src/jq# file jq
jq: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), statically linked, stripped
root@83a347734b03:/Users/wader/src/jq# ls -lh jq
-rwxr-xr-x 1 root root 1.1M Jul  2 14:04 jq
root@83a347734b03:/Users/wader/src/jq# ./jq --version
jq-1.6-215-gf88c4e5-dirty
root@83a347734b03:/Users/wader/src/jq# ./jq -n 1+2
3

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find! I'm going to merge this PR so that you have something to base off.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 sounds good

Yeap i think alpine for static binaries could be a good alternative, then i think we could truely have one binary that works on any linux kernel. @itchyny seemed a bit concerned about it but maybe can be convinced :) the alternative think is find a good distro and glibc version that produce known "good" static binaries, i guess that is was the previous releases did?

Out of curiosity had a look at the current jq 1.6 linux64 binary:

$ ldd jq-linux64
	not a dynamic executable
$ file jq-linux64
jq-linux64: ELF 64-bit LSB executable, x86-64, version 1 (GNU/Linux), statically linked, for GNU/Linux 2.6.32, BuildID[sha1]=c159e8a134ee222019dfbac0a1af4bc94b302eb0, with debug_info, not stripped

but couldn't find what libc variant/version is it, glibc something?

also notice that jq 1.6 had 32bit builds

Copy link
Member

@wader wader left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we merge this and iterate on it? i think this also fixes the macOS 12 CI errors we're seeing

@wader
Copy link
Member

wader commented Jul 2, 2023

Hmm any idea why "CI / release (pull_request)" was skipped and the release builds stuck at "Expected — Waiting for status to be reported" ... expected as in expected to be stuck waiting? :)

@owenthereal
Copy link
Member Author

Hmm any idea why "CI / release (pull_request)" was skipped and the release builds stuck at "Expected — Waiting for status to be reported" ... expected as in expected to be stuck waiting? :)

I have modified the requried actions to match this PR's. Now everything should be green :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants