git-arc: add initial non-FreeBSD and `$HOME` install support

[ngie]

Summary

Modify the existing code to permit setting a prefix via the PREFIX
make variable, similar to what --prefix provides in autotools-based
projects.

Add the required compatibility glue in order to function with vanilla
bmake, which enables installing the commands with Linux and macOS:

• vanilla bmake requires scripts.mk to install SCRIPTS.
• vanilla bmake does not tack /man on to the end of MANDIR, like with FreeBSD.

While here, update the origin for arcanist as the former is the
non-phorge fork is no longer supported (nor needed).

NOTE:

• regarding Linux support: this should now function out of the box with Linux as long as the host has access to an appropriate arc binary.
• regarding macOS support: the arcanist/phorge packages do not yet exist in Homebrew or [MacPorts][1], but are being added soon.

MFC after: 2 weeks

[1]: https://github.com/macports/macports-ports/pull/36217

Test Plan

• This continues to function on FreeBSD (it's how I posted this review).
• This code works when run against macOS:

➜  freebsd-src git:(gitarc-D60995) ✗ arc install-certificate
 CONNECT  Connecting to "https://reviews.freebsd.org/api/"...
LOG IN
Open this page in your browser and log in if necessary:

https://reviews.freebsd.org/conduit/login/

Then paste the API Token on that page below.

    Paste API Token from that page: ...
Writing ~/.arcrc...
 SUCCESS!  API Token installed.
➜  freebsd-src git:(gitarc-D60995) ✗ git arc update HEAD    

Does this look OK? [y/N] y
Launching editor "nvim"...
Update the revision comments, then save and exit.
 SKIP STAGING  No staging area is configured for this repository.
Updated an existing Differential revision:
        Revision URI: https://reviews.freebsd.org/D60995

Included changes:
  M       tools/tools/git/Makefile
  M       tools/tools/git/git-arc.sh
➜  freebsd-src git:(gitarc-D60995) ✗ sw_vers 
ProductName:		macOS
ProductVersion:		26.7.1
BuildVersion:		25G241

[ngie]

I reverted my accidental push to :main :(...

[ngie]

tools/tools/git/Makefile

3

The reason for the ?= removal is I was trying to get folks to rely on ${PREFIX} to install files to the correct directory hierarchy instead of using a bunch of make variables to achieve the same thing.

I need to do the same with MANDIR for symmetry.

[markj]

tools/tools/git/git-arc.sh

149–166

Doesn't this invalidate the comment immediately above?

[ngie]

Make MANDIR non-configurable/rely on ${PREFIX} for symmetry with the change to ${BINDIR}

This is being done to allow users to issue the [shorter] command...

% make install MK_INSTALL_AS_USER=yes PREFIX="$HOME/.local"

Instead of:

# On FreeBSD.
% make install MK_INSTALL_AS_USER=yes BINDIR="$HOME/.local/bin" MANDIR="$HOME/.local/share/man/man"
# On Linux/macOS
% make install MK_INSTALL_AS_USER=yes BINDIR="$HOME/.local/bin" MANDIR="$HOME/.local/share/man"

[ngie]

tools/tools/git/git-arc.sh

149–166

Doesn't this invalidate the comment immediately above?

Fair point - it does.

I'll think about this a minute before continuing...

[jrtc27]

tools/tools/git/Makefile

3

Why force your non-standard ideology on people when you can allow them to do things the standard way too if they want to?

[ngie]

tools/tools/git/git-arc.sh

149–166

Doesn't this invalidate the comment immediately above?

Fair point - it does.

I'll think about this a minute before continuing...

Ok, I (sort of) figured out a better way for handling this using pkg query. One minute...

[dteske]

tools/tools/git/git-arc.sh

148–149

149–166

In D59130 we were floating the idea of adding 2> /dev/null to the sysctl call.

I'd prefer:

: "${LOCALBASE:=$(sysctl -n user.localbase 2>/dev/null)}"
: "${ARC_CMD:=${LOCALBASE}/lib/php/arcanist/bin/arc}"

On FreeBSD this would keep the spirit of allowing devel/*arcanist-lib to be installed at the same time as archivers/arc without git-arc incorrectly trying to use /usr/local/bin/arc or trying to rely on pkg query (which usually doesn't exist on Linux or macOS)

Not to mention, no need to add a whole bunch of complexity given someone can:

export ARC_CMD=/path/to/arc

In a shell profile

[ngie]

• Use pkg query to restore the ability for archivers/arc and devel/freebsd-git-tools to be run on the same system (in theory - see D60998)
• Don't bother forking, etc, if ARC_CMD is already set via the environment.
• Don't hardcode the path to the arc binary in devel/phorgeit-arcanist-lib - this logic has a slightly better chance of getting the "right answer" once concurrent php flavors are permitted on the ports side.

[ngie]

tools/tools/git/git-arc.sh

149–166

In D59130 we were floating the idea of adding 2> /dev/null to the sysctl call.

I'd prefer:

: "${LOCALBASE:=$(sysctl -n user.localbase 2>/dev/null)}"
: "${ARC_CMD:=${LOCALBASE}/lib/php/arcanist/bin/arc}"

The problem with querying user.localbase is that if someone built the package with a --prefix != /usr/local, this logic would fail today--requiring the end-user to specify LOCALBASE, which could fail if git-arc adopts the variable further.

Scale that out to N developers if this kind of a change is deployed, and it gets hairy having to manage all the user configs. Besides, the less user configs where I have to apply the logic, the better: I could reimage them more frequently, which would allow me to treat my machines more like cattle and less like pets without investing in an infrastructure-as-code solution like ansible, chef, etc.

On FreeBSD this would keep the spirit of allowing devel/*arcanist-lib to be installed at the same time as archivers/arc without git-arc incorrectly trying to use /usr/local/bin/arc or trying to rely on pkg query (which usually doesn't exist on Linux or macOS)

Checking for pkg before calling pkg-query like I now have in the proposed diff is easy enough :).

Not to mention, no need to add a whole bunch of complexity given someone can:

export ARC_CMD=/path/to/arc

In a shell profile

It's a bit more tricky with Homebrew and MacPorts where they do some really curious things with how they use --prefix and link things in to /opt/local (MacPorts or Homebrew when arm64) or /usr/local (if Homebrew on Intel). When I get formulae (Homebrew) / portfiles (MacPorts) created upstream for the phorgeit packages, there is a good chance the respective packaging ecosystems permit building and installing with multiple versions of packages and multiple flavors of php (they already do flavors/variations a bit better than FreeBSD ports does, in part due to inertia, but also automation and being able to leverage existing infrastructure like GitHub, etc).

It's possible, but at the end of the day it would require folks to know the right commands with their appropriate package managers, and then placing that logic in their user shell rcs. Again, not impossible, but it makes dev machine customization more annoying and error prone.

[ngie]

tools/tools/git/Makefile

3

Why force your non-standard ideology on people when you can allow them to do things the standard way too if they want to?

These patterns are similar to what I pushed upstream with git-pr: https://codeberg.org/FreeBSD/git-pr .

I am trying to make it easier for folks to use the tools without having to know every variable under share/mk (a README.md is pending for this directory). TBH, bikeshedding about how things are done with Phabricator/Phorge isn't a good use of either of our time, so I'm ok with restoring the previous behavior (add ?= back to BINDIR and MANDIR, enabling the prior workflow).

[jrtc27]

tools/tools/git/Makefile

3

Yes that’s what I’m asking for,
I’m fine with PREFIX existing so long as each specific derived variable can still be overridden the normal way

[dteske]

tools/tools/git/git-arc.sh

149–166

All good points and well taken. Seems very well thought-out. Thank you for improving the code and finding the edge-case that I had not considered.

I do have some qualms with passing -E to grep that does not use an extended regex. The -E is not required in this case.

[ngie]

tools/tools/git/git-arc.sh

149–166

All good points and well taken. Seems very well thought-out. Thank you for improving the code and finding the edge-case that I had not considered.

I do have some qualms with passing -E to grep that does not use an extended regex. The -E is not required in this case.

Good point: will remove the -E.

[ngie]

Address comments from @dteske and @jrtc27.

[ngie]

Missed @dteske's comment update suggestion in prior update.

[dteske]

tools/tools/git/git-arc.sh

152–153

1. The backslash after a pipe at the end of a line is unnecessary
2. list reads better than query %Fp -- both have identical output
3. You can safely remove the quotes around /bin/arc$

This all means you can squish it all on one line that is less than 80c

[ngie]

Address additional comments from @dteske.

[dteske]

Thank you again for taking the time to improve the code

I have just the last final concerns/nits.

1. What if someone installed from source or made their own pkg? Meaning file exists but “pkg which path” shows either that pkg has no record of it or that the package that owns it is not PHORGE_PACKAGE? In that latter case, we don’t even allow overriding PHORGE_PACKAGE.
2. GIT_ARC_PACKAGE seems like it would be a better name
3. Invoking pkg each time seems like a bit of a heavy handed solution. I support referencing pkg, but it seems like a bit of a regression to go straight to pkg as the first and only solution for anyone that has the thing installed (maybe not by pkg) and also has archives/arc installed (muddying the waters with unrelated arc in PATH)

All this is to say, I understand the edge case that the sysctl query has, but I still would like to see it performed as a first check to prevent a regression in behavior. That we can, and should support querying pkg, but we should also allow inheriting a value for PHORGE_PACKAGE, but perhaps we should make that variable a bit more specific. For example, what about GIT_ARC_PACKAGE (being explicit to your prior points about how LOCALBASE could effect many things)

[ngie]

In D60995#1399215, @dteske wrote:

Thank you again for taking the time to improve the code

I have just the last final concerns/nits.

1. What if someone installed from source or made their own pkg? Meaning file exists but “pkg which path” shows either that pkg has no record of it or that the package that owns it is not PHORGE_PACKAGE? In that latter case, we don’t even allow overriding PHORGE_PACKAGE.

That feels outside the scope of this change. If someone's rolling their own version of arcanist, they need to set the value appropriately.

2. GIT_ARC_PACKAGE seems like it would be a better name

No. arc and git-arc are distributed in two different packages:

➜  git pkg query '%Fp %o' | grep -E '/bin/(git-)?arc'
/usr/local/bin/git-arc devel/freebsd-git-devtools
/usr/local/lib/php/arcanist/bin/arc devel/phorgeit-arcanist-lib
/usr/local/lib/php/arcanist/bin/arc.bat devel/phorgeit-arcanist-lib
/usr/local/bin/arc devel/phorgeit-arcanist

I will bite on ARC_PACKAGE.

3. Invoking pkg each time seems like a bit of a heavy handed solution. I support referencing pkg, but it seems like a bit of a regression to go straight to pkg as the first and only solution for anyone that has the thing installed (maybe not by pkg) and also has archives/arc installed (muddying the waters with unrelated arc in PATH)

Eh... Is it really a heavyhanded solution?

The freebsd-git-tools package should really be modifying the path inline as part of the package build to specify whatever --prefix is for the package. Querying ${LOCALBASE} seems like a hack.

If folks are installing the package from their source tree instead of installing the package... that seems like a very specific edgecase that is outside the scope of concern.

All this is to say, I understand the edge case that the sysctl query has, but I still would like to see it performed as a first check to prevent a regression in behavior. That we can, and should support querying pkg, but we should also allow inheriting a value for PHORGE_PACKAGE, but perhaps we should make that variable a bit more specific. For example, what about GIT_ARC_PACKAGE (being explicit to your prior points about how LOCALBASE could effect many things)

Again, this seems like it's being overthought. 99% of developers should install the package; the only time I really see there being value in installing from the source tree is when doing quick checks to make sure the code works; TBF, a lot of the stuff in tools/tools/git should really be managed in other repos and packages should be created by ports instead of having a copy of the tools bitrotting in the src tree (tools/tools/git/ghpr: I'm looking straight at you).

[dteske]

I can agree on ARC_PACKAGE and allowing inheritance.

I disagree on the two points of (1) scope and (2) overthinking.

My points are of the simple matter of POLA.

Regardless of how you feel about the way the tool used to work, an upgrade should not leave the user astonished that their setup is now broken and requires setting a variable.

I believe very strongly that the change should not break anyone’s access, and the current proposal very well could. I outlined the edge cases that the current pkg based approach could leave someone reeling after an upgrade. So, not out of scope to try and avoid breaking what already works today.

[ngie]

Restore prior FreeBSD behavior

[ngie]

Fix syntax error (fi -> esac)

[ngie]

tools/tools/git/git-arc.sh

152–153

1. The backslash after a pipe at the end of a line is unnecessary
2. list reads better than query %Fp -- both have identical output
3. You can safely remove the quotes around /bin/arc$

This all means you can squish it all on one line that is less than 80c

I just dropped the pkg checks.

I don't want to debate the prior method I suggested using pkg, POLA violations, etc. I have enough other stuff on my plate and all I really cared about was adding macOS support.

[ngie]

Diff reduce

[markj]

tools/tools/git/git-arc.sh

105

These messages are FreeBSD-specific.

The man page still refers to devel/arcanist-lib.

163

[ngie]

Make documentation/error messages more generic

[ngie]

Be clear that devel/freebsd-git-arc is only available on FreeBSD.

[ngie]

Add missing ..

[ngie]

tools/tools/git/git-arc.sh

105

These messages are FreeBSD-specific.

The man page still refers to devel/arcanist-lib.

I updated the manpage and script to make the error message/dependencies more generic.