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

Update documentation and installation scripts, fix bug #20

Merged
merged 7 commits into from
Feb 17, 2023
Merged

Conversation

vdye
Copy link
Collaborator

@vdye vdye commented Feb 16, 2023

Closes #15

This pull request updates a number of installation-related scripts, focusing primarily on documentation.

  • The first commit fixes a bug in launchd.go I somehow missed in all of my prior testing of the service 🙃
  • The next three commits refactor the install/uninstall scripts to generalize them for packaging and install-from-source applications, including adding an install target to the Makefile.
  • The next two commits add Asciidoc pages for git-bundle-server and git-bundle-web-server including custom Asciidoctor extensions for some specialized macros and a README.md containing information about how the documentation should be updated. These documents are built into man pages with make doc and are installed (with appropriate symlinks) in all supported installation methods.
  • The last commit adds detailed install/uninstall instructions to the repository's README.md, and does some minor reorganization of the file.

Testing

Manual installs of packages and from source on both MacOS (amd64) and Ubuntu. Successful pipeline run: https://github.com/github/git-bundle-server/actions/runs/4189915807

vdye added 3 commits February 14, 2023 15:37
If a 'launchd' service is bootstrapped but not currently running, calling
'launchctl kill SIGINT' on the service will result in error code 3 ("No such
process"). Stopping an already-stopped service isn't an error case, so
update to ignore the error code.

As part of adding a new test to cover this error code, reorganize
'TestLaunchd_Stop' into table-driven tests to reduce code duplication.

Signed-off-by: Victoria Dye <vdye@github.com>
The options present in the 'ln' command differ between Linux and MacOS ('-r'
in the former creates a relative link from absolute paths, '-F' in the
latter forces overwriting an existing link). Drop use of these options in
packaging/installation scripts by instead explicitly specifying relative
paths and deleting existing links before calling 'ln', respectively.

Signed-off-by: Victoria Dye <vdye@github.com>
To prepare for an install-from-source target in the Makefile, create a
general (Unix) 'uninstall.sh' script. This script is nearly identical to
'build/package/pkg/uninstall.sh', with a few notable differences:

1. The script does not run as 'root' by calling itself with 'sudo'; instead,
   operations that may require root privileges are first tried as the
   calling user and, if they fail, are called again with 'sudo'.
2. If the script is run with 'sudo' (or otherwise as 'root'), the user is
   prompted to confirm whether they want to proceed.

These changes help us avoid platform-specific code around finding the
current logged in user to run 'git-bundle-server web-server stop'.

Signed-off-by: Victoria Dye <vdye@github.com>
@vdye vdye self-assigned this Feb 16, 2023
@vdye vdye marked this pull request as ready for review February 16, 2023 01:48
@vdye vdye requested a review from jeffhostetler February 16, 2023 16:38
internal/daemon/launchd.go Show resolved Hide resolved
Makefile Show resolved Hide resolved
docs/man/git-bundle-server.adoc Show resolved Hide resolved
docs/man/git-bundle-server.adoc Show resolved Hide resolved
README.md Show resolved Hide resolved
Copy link
Collaborator

@ldennington ldennington left a comment

Choose a reason for hiding this comment

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

Looks good! Especially liked the consolidation down to the single uninstall.sh script. One thing I noticed is that the ruby asciidoctor extension does seem to be experimental, but if you've been using it successfully I don't think that's a big deal.

scripts/install.sh Outdated Show resolved Hide resolved
docs/man/README.md Outdated Show resolved Hide resolved
docs/man/git-bundle-server.adoc Show resolved Hide resolved
Add an install-from-source target to the 'Makefile' to allow installation on
any Unix-based system, rather than only those for which packages are built
(i.e., Darwin and Debian). This is done by repurposing the 'layout-unix.sh'
script into an 'install.sh' script. Like the 'uninstall.sh' update from the
previous commit, the new script contains only a few major differences from
'layout-unix.sh':

- The '--payload' option is renamed to '--install-root'.
- Like 'uninstall.sh', commands that may require root privileges (such as
  when '--install-root=/') are wrapped in a 'retry_root' function that first
  runs the operation as the calling user, then as root if it fails. However,
  unlike 'uninstall.sh', this behavior is gated by an '--allow-root' option,
  ensuring we only do this escalation if we're running the 'install' target.
- 'INSTALL_TO' is renamed to 'APP_ROOT' to avoid confusion with
  'INSTALL_ROOT'. This is also done in the pkg 'postinstall' script.

Signed-off-by: Victoria Dye <vdye@github.com>
vdye added 3 commits February 16, 2023 15:41
Add Asciidoc manual pages for 'git-bundle-server' (describing bundle servers
in general as well as the tool's individual commands) and
'git-bundle-web-server'.

In addition to the Asciidoc pages, add custom 'man:' and 'url:' macros in
'asciidoctor-extensions.rb'. The former is a simplified version of an
Asciidoctor example extension [1] (providing basic markup for references to
other man pages), while the latter overrides the default autolinking [2]
behavior in 'asciidoctor' to avoid some roff rendering weirdness and applies
some simple markup.

These files are intended for use with Asciidoctor. Documents can be
generated with:

$ asciidoctor -I scripts/ -r asciidoctor-extensions.rb docs/man/*.adoc

In a later commit, these docs will be built into installable manpages as
part of a 'Makefile' target.

[1] https://github.com/asciidoctor/asciidoctor-extensions-lab/blob/main/lib/man-inline-macro/extension.rb
[2] https://docs.asciidoctor.org/asciidoc/latest/macros/autolinks/

Signed-off-by: Victoria Dye <vdye@github.com>
Add a 'doc' target and 'make-docs.sh' script to build the Asciidoc documents
in 'docs/man' into installable manpages. Include these documents (and
associated symlinks) in installations from source and from platform-specific
packages.

Additionally, update 'release.yml' to install 'asciidoctor' so that the
documentation can successfully be built in that workflow.

Signed-off-by: Victoria Dye <vdye@github.com>
Add instructions for installing/uninstalling the bundle server. Update
instructions for local development to suggest using 'make' and to
recommended a 'bin/' output directory when using 'go build' directly.

Signed-off-by: Victoria Dye <vdye@github.com>
@vdye vdye merged commit 50133e5 into main Feb 17, 2023
@vdye vdye deleted the vdye/docs branch February 17, 2023 16:16
@vdye vdye added this to the v1.0 milestone Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Write more detailed usage documentation
3 participants