git-ecosystem / git-bundle-server

A web server & management CLI to host Git bundles for use with Git's "bundle URIs" feature
Other
43 stars 20 forks source link

Create Makefile & release pipeline, make `git-bundle-server web-server` more resilient #12

Closed vdye closed 1 year ago

vdye commented 1 year ago

Summary

This series is made up of a few loosely-connected changes:

Prior Art

microsoft/git - build-git-installers.yml

This release pipeline is the source of both the variable calculation in the prereqs step and the create-github-release step. Note that the latter is updated to handle Promises a bit more carefully, uses a new version of github-script (and therefore changes references of github to github.rest^1), and includes an intermediate step to move artifacts into a single directory before upload.

GitCredentialManager/git-credential-manager - misc. scripts (1, 2, 3)

The scripts found in build/package here are derived from those linked in GCM. There were a lot of minor tweaks to adjust to this codebase, but some major differences include:

Testing

Successful pipeline run

Draft release (screenshot)

image

Other

I've locally built, installed, and uninstalled the packages on both my (Intel) Mac and an Ubuntu Codespace.

If you get the chance, I'd love it if y'all could try installing the generated artifacts (found here), especially if you use an M1 Mac. To see if the install worked, run git-bundle-server -h and you should get help text.

Future work

derrickstolee commented 1 year ago

(I co-assigned myself to remind me to review this, but I won't have time until next week.)

jeffhostetler commented 1 year ago

um, WOW!!! Let me just stop there.

I want to steal so much of this for my thing... :-)

I promise I'll give it a more "critical" review shortly.

jeffhostetler commented 1 year ago

It installed successfully on my M1 mac. The git-bundle-server -h command works.

jeffhostetler commented 1 year ago

However, when running the uninstall script, I got an error. (I'm not sure that the web-server was running.)

m1 ~/work/git.git % ps auwx | grep git-bun
jeff             12392   0.0  0.0 407962240    176 s018  R+    3:24PM   0:00.00 grep git-bun
m1 ~/work/git.git % sudo /usr/local/git-bundle-server/uninstall.sh 
Password:
Stopping the web server daemon for user 'jeff'...
2023/02/03 15:24:47 Failed with error: could not unload launchd service: 'launchctl bootout' exited with status 5
m1 ~/work/git.git % 

And the files are still in /usr/local/

vdye commented 1 year ago

However, when running the uninstall script, I got an error. (I'm not sure that the web-server was running.)

Welp, that's what I get for not testing the uninstall script properly - I tested running it after starting the web server, so the plist was populated. I need to add a check that the file exists - if it doesn't, bootout fails with error 5 ("Input/Output error"), rather than 113 as I assumed.

The more annoying thing is that you get a different, more informative error code when running launchctl as root (error 2, "No such file or directory"), but I also don't want to run as root if I can avoid it. I'll figure something out, thanks for testing!

vdye commented 1 year ago

@jeffhostetler I've made some changes to git-bundle-server web-server stop --remove, so now it shouldn't fail if the plist hasn't been created/the server hasn't been started. If you want to try again, these artifacts should be able to install & uninstall without the error you saw.

jeffhostetler commented 1 year ago

This one installed and uninstalled just fine.

(I never started the web-server, so this was a minimal test at best.)

vdye commented 1 year ago

Update: after testing on an Ubuntu VM, I found that 1) some fixes I had made to the Debian pack.sh & layout-unix.go got reverted at some point 🙃 and 2) I needed to add handling in systemd.go for when the service is stopped before it's ever started (i.e., the service unit doesn't exist on disk). I think the easiest way to show the changes is with a range diff since e971c13d0de02a2280997418e36ae2045372b31b:

Range diff ```diff -: ------- > 1: 374758a systemd: do not fail 'stop' if service unit not installed 1: 4e4341c = 2: 02282c4 launchd: enable running after user logs out 2: 008d5de = 3: 8c6a468 launchd: unload by service target rather than filename 3: 7645b2f = 4: c82b8e0 web-server: add '--remove' option to 'stop' 4: d656905 = 5: 7660c16 web-server: rename daemon to align with package naming 5: 3371111 = 6: be8f1ea Makefile: create basic 'build' and 'clean' targets 6: 0fb5df8 ! 7: 18c1ddc Makefile: add Debian packaging targets @@ Makefile: build: + @echo + @echo "======== Creating binary Debian package ========" + @build/package/deb/pack.sh --payload="$(DEBDIR)/root" \ ++ --scripts="$(CURDIR)/build/package/deb/scripts" \ + --arch="$(PACKAGE_ARCH)" \ + --version="$(VERSION)" \ + --output="$(DEB_FILENAME)" @@ build/package/deb/pack.sh (new) + DEBROOT="${i#*=}" + shift # past argument=value + ;; ++ --scripts=*) ++ SCRIPT_DIR="${i#*=}" ++ shift # past argument=value ++ ;; + --arch=*) + ARCH="${i#*=}" + shift # past argument=value @@ build/package/deb/pack.sh (new) +mkdir -p "$(dirname "$DEBOUT")" + +# Build .deb -+mkdir -p "$DEBROOT/DEBIAN" ++mkdir -m 755 -p "$DEBROOT/DEBIAN" + +# Create the debian control file +cat >"$DEBROOT/DEBIAN/control" <

The good news is that I've confirmed the user-scoped service in systemd keeps running after logging out, so we don't need to mess with any root-privileged settings there. I've also successfully installed and uninstalled there, checking the behavior both when the web-server is never started, and when it has been started.

EDIT: and here's a new release build

derrickstolee commented 1 year ago

I tested this on my Intel Mac and installation succeeded without issue. I ran into #13, so I added my error message to that description.

After fixing the missing directory issue, I did get this prompt:

image

I suppose this is inevitable for the macOS ecosystem, but something to be aware of, right?

vdye commented 1 year ago

I suppose this is inevitable for the macOS ecosystem, but something to be aware of, right?

That's interesting, I haven't seen that notification (whether I install from a package built locally or from the one generated by the release pipeline). The only one I've encountered is this one when starting the web server:

image

My guess would be that it's something cron related, especially if you saw it after running init. I'll keep an eye out for that and other errors as I keep testing.

jeffhostetler commented 1 year ago

I've had to add git-telemetry-service manually to my firewall settings. It's well hidden under the "options..." page on the "firewall" page. However, it looks like there might be a way to add an exception/open a port from the command line. I haven't played with it yet. See man pfctl and https://www.macworld.com/article/671729/mac-firewall-how-to-open-specific-ports-in-os-x-10-10-firewall.html

jeffhostetler commented 1 year ago

Hey, just found this while working on a preview package (and stealing generously from this PR). There's an option to go build to let you inject a version number into the source. I though you might be interested.

With source looking like this:

% more main_version.go 
package main

// This value will be overwritten during release builds.
var MainVersion string = "v0.0.0"

We can force a new value at compile time using:

go build -ldflags="-X 'main.MainVersion=v1.2.3'"

I'm still experimenting, but it looks promising.