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

Add nightly builds #1648

Merged
merged 10 commits into from
Nov 11, 2020
Merged

Conversation

steve-chavez
Copy link
Member

@steve-chavez steve-chavez commented Nov 9, 2020

Fixes #1645. Adds nightly builds for Linux, Windows, OSX, FreeBSD. The results would look like this(using my own repos for the examples):

Reasoning

A tag for every nightly build would spam the Github releases page, and that would make it harder to see the project progress linearly.

An alternative would be to have a separate repo for the nightlies, but this would mean more maintenance work as we'd have to keep the repos in sync.

So all the nightly builds will be added on a single release. For dockerhub, new tags will be created.

Workflow

The workflow is partly manual and partly automated. These manual steps are required for releasing a nightly:

git tag -f nightly
git push <remote> nightly -f

Then all of the CI's process should kick in and release the nightlies.

I think the workflow is good because not every commit(refactors, ci changes, readme, etc) would need a nightly.

@wolfgangwalther
Copy link
Member

wolfgangwalther commented Nov 9, 2020

Didn't like the idea of spamming the GitHub releases page and creating tags with every build, so all the nightly builds will be added on a single release. For dockerhub, new tags will be created.

Hm. I think it makes sense not to create GitHub releases, but it would still be great to have the nightly builds tagged on github the same way they are on dockerhub as well. This would make it a lot easier to track down the current commit for each nightly - and the other way around as well.

Maybe it's possible to create this tag from the CI workflow, where the docker tag is created and then push it back to github as well?

@steve-chavez
Copy link
Member Author

Hm. I think it makes sense not to create GitHub releases

@wolfgangwalther Just in case, I'll elaborate :). From what I've seen, the usual practice for nightlies is to create another repo and do the releases there. Like: https://github.com/OoliteProject/nightlies/releases

Creating another repo would mean more maintenance, that's why I opted out of that option. Now, since all the releases would be on the releases page, having many nightlies alongside the stable releases would make the project changes harder to explore. That's what I meant with "spamming".

but it would still be great to have the nightly builds tagged on github the same way they are on dockerhub as well.
This would make it a lot easier to track down the current commit for each nightly - and the other way around as well.

What workflow would be inconvenient without a nightly tag?

Even without the tag, the user reporting an issue has the <sha> in the binary name. So when an issue is reported, we just do git checkout sha to locate on the commit belonging to the nightly.

Maybe it's possible to create this tag from the CI workflow, where the docker tag is created and then push it back to github as well?

I had some issues with creating a tag from CI(since it triggers another build), but I guess it'd be a matter of blacklisting the tags.
Also, what I thought was that if we create the tags then we might as well have the binaries on each tag, and then we'd go back to the "spam" issue.

@wolfgangwalther
Copy link
Member

What workflow would be inconvenient without a nightly tag?

Even without the tag, the user reporting an issue has the <sha> in the binary name. So when an issue is reported, we just do git checkout sha to locate on the commit belonging to the nightly.

That was exactly what I was thinking of. Is this possible with the docker images as well? The last bit of the docker-tag is the sha, right? I missed that before.

@steve-chavez
Copy link
Member Author

steve-chavez commented Nov 9, 2020

Yes! Also possible with docker, it's the same sha. Actually it's the short sha, maybe that's why it wasn't obvious at first.

@wolfgangwalther So what do you think? Is the nightly builds setup good as it is now?

@wolfgangwalther
Copy link
Member

Looking at this link you gave in the initial post:

https://github.com/steve-chavez/postgrest/releases/tag/nightly

The freebsd binarys are only a couple 100 bytes in size. Is that correct? Or already fixed?

@wolfgangwalther
Copy link
Member

Well two of them are, the third one is fine. But it's odd, because it's not in chronological order.

@steve-chavez
Copy link
Member Author

The freebsd binarys are only a couple 100 bytes in size. Is that correct? Or already fixed?

Yeah, those were wrong, but it's already fixed. cabal v2-install copied a symlink to the /.cabal/bin dir. So tar stored the symlink. Adding tar .. --dereference ... made tar follow the symlink.

@steve-chavez
Copy link
Member Author

But it's odd, because it's not in chronological order.

That's true. I thought it'd be rare to have more than one nightly per day.

To ensure the order of the builds I could add the time date +%H:%M to the binaries names. So they'd look like:

postgrest-nightly-2020-11-08-12:13-abf063d-freebsd.tar.xz

WDYT? Would that be better?

@wolfgangwalther
Copy link
Member

@wolfgangwalther So what do you think? Is the nightly builds setup good as it is now?

Yes. Love it. Went through everything: LGTM.

@wolfgangwalther
Copy link
Member

wolfgangwalther commented Nov 9, 2020

But it's odd, because it's not in chronological order.

That's true. I thought it'd be rare to have more than one nightly per day.

To ensure the order of the builds I could add the time date +%H:%M to the binaries names. So they'd look like:

postgrest-nightly-2020-11-08-12:13-abf063d-freebsd.tar.xz

WDYT? Would that be better?

It certainly does not hurt and in the case of multiple nightlys it will help. So yes. But don't use a :, that's invalid on Windows.

And of course it would make sense to then add this to the docker image as well! (But that's probably want you would do anyway)

@wolfgangwalther
Copy link
Member

How does the nightly binary identify itself?

I just tried the following:

  • the docker image for 7.0.1 returns "7.0.1 (UNKNOWN)" in the api description
  • my current local build returns "7.0.1 (3cde1cd)" when on the command line USAGE screen.

can we fix both the UNKNOWN and make the 7.0.1 a "nightly" or "7.0.1-nightly" for the new builds?

@steve-chavez
Copy link
Member Author

To ensure the order of the builds I could add the time date +%H:%M

Actually I'm not sure if this would be a good idea. The builds could finish at a different minutes and we'd have different names for the same binary on different platforms. I guess just append the hour? Or maybe just keep it with the sha..

can we fix both the UNKNOWN

Regarding UNKNOWN, that was fixed on #1533.

How does the nightly binary identify itself?

That's important, I overlooked it. What I thought now is to modify the postgrest.cabal version field at runtime and insert the same nightly-$suffix and then generate the binary with cabal v2-install.

Unfortunately it seems version only accepts digits and it errs with:

Errors encountered when parsing cabal file ./postgrest.cabal:

postgrest.cabal:2:21: error:
unexpected "n"
expecting white space or version digit (integral without leading zeroes)

    1 | name:               postgrest
    2 | version:            nightly-2020-11-08-abf063d

The workaround I'm seeing now is to only use the date:

version:            20201108

So this would generate:

cabal v2-run postgrest

Missing: FILENAME

Usage: postgrest FILENAME
  PostgREST 20201108 (0e46740) / create a REST API to an existing Postgres database

@wolfgangwalther
Copy link
Member

Actually I'm not sure if this would be a good idea. The builds could finish at a different minutes and we'd have different names for the same binary on different platforms. I guess just append the hour? Or maybe just keep it with the sha..

Very good point. This can happen with only the date as well, though. It'd be better to take the timestamp of the commit in question on all platforms. Something like the following maybe?

git show -s --format="%cd-%h" --date="format:%Y-%m-%d-%H-%M" v7.0.1
# output is:
# 2020-05-18-13-05-d6050c8

Just replace v7.0.1 with nightly. Short hash included :)

That's important, I overlooked it. What I thought now is to modify the postgrest.cabal version field at runtime and insert the same nightly-$suffix and then generate the binary with cabal v2-install.

Unfortunately it seems version only accepts digits and it errs with:

Errors encountered when parsing cabal file ./postgrest.cabal:

postgrest.cabal:2:21: error:
unexpected "n"
expecting white space or version digit (integral without leading zeroes)

    1 | name:               postgrest
    2 | version:            nightly-2020-11-08-abf063d

The workaround I'm seeing now is to only use the date:

version:            20201108

So this would generate:

cabal v2-run postgrest

Missing: FILENAME

Usage: postgrest FILENAME
  PostgREST 20201108 (0e46740) / create a REST API to an existing Postgres database

I played around with it a little bit. 9 digits seems to be a limit as well, so hours and minutes would not fit in. Also dots in between don't work, because a single digit can't have a leading zero.

Just Ymd is fine, since it doesn't affect any sorting order here.

This is what I came up with, in case it helps:

sed -i -r "s/^(version:\s+)\S+$/\1$(git show -s --format='%cd' --date='format:%Y%m%d' nightly)/" postgrest.cabal

@steve-chavez
Copy link
Member Author

@wolfgangwalther Thanks a lot for the pointers :). I had some issues with sed not working the same on FreeBSD/OSX(-r/-E didn't work, sed -i needs a prefix), but it all seems to be working good now.

I played around with it a little bit. 9 digits seems to be a limit as well, so hours and minutes would not fit in.

Btw, regarding the cabal nightly version. I think it might be possible to insert a custom X-nightly-version: nightly-$suffix in postgrest.cabal and then use this instead of the version for the api docs and program usage.

I got the idea by seeing this lib and the customFieldsPD on PackageDescription. Maybe it's also possible by having a build time option: cabal v2-build --configure-option=OPT(not sure). Those options could be explored later.

For now, I'll merge and test releasing a nightly 🌃 .

@@ -12,35 +12,28 @@ let
# Version from the postgrest.cabal file (gotten with callCabal2nix).
version = postgrest.version;

# Set of files that will be published in the GitHub release.
releaseFiles =
runCommand "postgrest-release-files"
Copy link
Member Author

Choose a reason for hiding this comment

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

@monacoremo In case you're wondering. I deleted this nix function because I couldn't figure out how to pass a bash argument to it in writeShellScriptBin "postgrest-release-github". My nix-fu isn't that good 😄.

''
set -euo pipefail

mkdir -p $out

tar cvJf "$out"/postgrest-v"$version"-linux-x64-static.tar.xz \
Copy link
Member Author

Choose a reason for hiding this comment

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

I've removed the v prefix from v$version in the scripts. This means that the changelog versions will now need a v prefix for sed to pick the changes and then serve as the body for the release(done with ghr).

@steve-chavez steve-chavez merged commit 3830887 into PostgREST:master Nov 11, 2020
@steve-chavez
Copy link
Member Author

The builds worked! The only failure was appveyor: https://ci.appveyor.com/project/postgrest/postgrest/builds/36245638, but that seems to be a ghr bug(maybe a race condition). The postgrest.exe artifact was already there so I released it manually.

There's an issue with the Linux build though. The binary is showing the 7.0.1 on postgrest --help. Of course, all the new additions are there.

I've checked the windows binary and it does show the 20201110 on postgrest --help.
I'm assuming the same for OSX and FreeBSD because their logs show: Configuring postgrest-20201110....

Checking the nix-build-test job on circle shows a Configuring postgrest-20201110 and also Building and caching all derivations...(pushing to cachix).

So there must be something wrong with the release job. Maybe cachix is not getting the previous job push?

@wolfgangwalther
Copy link
Member

wolfgangwalther commented Nov 11, 2020

There's an issue with the Linux build though. The binary is showing the 7.0.1 on postgrest --help. Of course, all the new additions are there.

Yes, I can confirm that with the docker image. Also it does not show the commit hash but just the version. Is that what was expected from #1533? It would be really good, if the docker image was showing the hash as well.

Checking the nix-build-test job on circle shows a Configuring postgrest-20201110 and also Building and caching all derivations...(pushing to cachix).

In that log it says:

compressing and pushing /nix/store/myf0g9hpi3mxz1srky6xbp8kymxr0zrx-docker-image-postgrest.tar.gz (3.90 MiB)

So there must be something wrong with the release job. Maybe cachix is not getting the previous job push?

Here it says:

copying path '/nix/store/vzzgrg9y512kh8izf28wgg00zqygxs00-docker-image-postgrest.tar.gz' from 'https://postgrest.cachix.org'...

This hash is created and pushed here:

https://app.circleci.com/pipelines/github/PostgREST/postgrest/480/workflows/35f226b6-ea37-480d-bad1-7a4b06bcb7cd/jobs/6996

This was the last commit you merged before.

So the timeline was like this:

  • circleci run 480 (commit: Add test to insert into a view and expect location header return) build this docker image
  • circleci run 483 (commit: ci: add nightly version to cabal file) was building the new docker image
  • circleci run 484 (tag: nightly) was building the same image as 483, but they were running in parallel

So when 484 pushed it's image to docker, it was using the build from the pipeline before, in this case 480, because 483 wasn't finished yet.

There is certainly something wrong with the release pipeline. You wouldn't have noticed it, though, if you had pushed the nightly tag after 483 had finished already. So maybe that's why it was not a problem so far.

@steve-chavez
Copy link
Member Author

steve-chavez commented Nov 11, 2020

Yes, I can confirm that with the docker image. Also it does not show the commit hash but just the version. Is that what was expected from #1533? It would be really good, if the docker image was showing the hash as well.

Hmm.. the docker image would have the same output as the linux static bin. So if I fix the 7.0.1 error, docker would show 20201108 as well. #1533 still requires the .git dir, so the commit hash won't be shown.

The only way I see for including the sha independently from .git is by doing the custom X-nightly-version I mentioned previously.


Remo emailed me about the release job problem. We need to patch the postgrest.cabal with the nightly version on the release job as well. I'll try this next.

@steve-chavez
Copy link
Member Author

steve-chavez commented Nov 13, 2020

We need to patch the postgrest.cabal with the nightly version on the release job as well.

This one worked!

I'll push the fix to master.

Edit: Had to delete the nightly tag on my github repo to prevent uploading the wrong tag to upstream.

@wolfgangwalther
Copy link
Member

wolfgangwalther commented Nov 14, 2020

#1533 still requires the .git dir, so the commit hash won't be shown.

I don't understand that bit. Isn't Development.GitRev supposed to compile the git hash into the executable? So the .git directory needs to be available at build time, right? The editing of the postgrest.cabal file we're currently doing is based on the .git directory (git show ...), the build environment should have this directory available?

So basically.. how can the edited postgrest.cabal file be in the build env, but not the .git directory that was there at the same time? #1533 mentions something related to downloading a tarball from hackage - but wouldn't this prevent the update to postgrest.cabal as well?

@monacoremo
Copy link
Member

@wolfgangwalther I think most of the confusion is from how nix works, which is not surprising as it's quite unusual - allow me to elaborate a bit:

The hash vzzgrg9... in a nix output path like /nix/store/vzzgrg9y512kh8izf28wgg00zqygxs00-docker-image-postgrest.tar.gz is a hash of absolutely all inputs required to build that derivation. This includes the PostgREST source code. We include that source code here, filtering out anything that might change arbitrarily, like the .git directory. We do include the .cabal file, though. Nix copies the source code into the /nix/store, naming the directory based on the hash of all included files.

When you run nix-build, the actual builds run in an isolated environment in temporary directories, not your current working directory. The source code is referred to by its new path in the nix store.

If anything in the source changes (like us patching the .cabal file), this results in a new hash of the input files and a new output hashed for all PostgREST build artefacts. This is why we need to also patch the .cabal file in the release job, otherwise nix will correctly pull the 7.0.1 version from cachix (or build anything missing).

@steve-chavez
Copy link
Member Author

Isn't Development.GitRev supposed to compile the git hash into the executable? So the .git directory needs to be available at build time, right?

@wolfgangwalther Right, I was mistaken in thinking that GitRev got the information from .git at runtime. I deleted the .git dir and cabal v2-run postgrest kept showing the the hash.

Maybe this is the real underlying issue with losing the git hash: acfoltzer/gitrev#23.

@wolfgangwalther
Copy link
Member

If I understood @monacoremo correctly, then the .git directory is already lost before cabal can loose it, because nix-build does not copy it to the build folder. So it seems like the only way to keep the hash is by passing it in another file, that is copied to those build environments... which comes down to your suggestion above, regarding X-nightly-version.

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

Successfully merging this pull request may close these issues.

Add nightly builds
3 participants