search

NitorCreations / license-maintainer

Maintains license preamble in source files etc in your project
Other
15 stars 11 forks source link

Adapted install script for compatibility with the macos ln command (hooks-only branch) #11

Closed alexclewontin closed 4 years ago

alexclewontin commented 4 years ago

The ln command on MacOS lacks a --backup flag, so the script fails. This commit modifies the install script to detect the operating system, and run the command to create the symlink without the --backup flag if it is MacOS.

I modified this for use in one of my projects, and figured I'd make it available to you for the base repository, if you are interested. Thanks for the great script!

xkr47 commented 4 years ago

Thanks, really appreciated! The backup thing was added to not lose any existing hooks possibly already there prior to installation. Would it perhaps be better if the script was changed to just fail if there are any conflicts and ask the user to manually do the installation instead in that case.. I'll think about this a bit.

xkr47 commented 4 years ago

Ok I decided to merge your PR now and possibly improve the instructions/script later. Thanks again for the contribution!

alexclewontin commented 4 years ago

Thanks for merging! Yeah I totally get the reasoning behind it and given the presence of the -backup flag, it makes total sense (as a side note, it bugs me every time I find a MacOS command line utility that is slightly incompatible with the Linux version's flags).

I have not actually experimented with this, but for what it's worth, I believe https://ss64.com/osx/ln.html says that without the -f flag, the install script will fail if there is a name conflict, so given that there isn't a -f flag currently I think it would exhibit the desired behavior as-is.

There's also an -i flag that will prompt the user for input in the case of a conflict, but the man page is kind of ambiguous about the behavior when encountering a conflict with a hard link/regular file vs a conflict with another symlink.

alexclewontin commented 4 years ago

Another way to handle it would be simply to do something like this:

echo "./.githooks/license-maintainer/pre-commit" >> .git/hooks/pre-commit.

Completely non-destructive, and cross-platform.

The problem I foresee here is that if the user has an already-configured hook in Python or Perl or what-have-you with a #!, appending a line that assumes shell script to the end will a) not work, and b) possibly break the existing hooks. You could check to make sure the file is shell script, but that's non-trivial.

EDIT: You'd also probably need to detect whether there is already a file with a #!/bin/sh,#!/usr/bin/sh, #!/usr/bin/bash etc. so you can add it if necessary. Which then we get back into the problem of identifying conflicts that we have currently with ln. So this might in fact be worse.

xkr47 commented 4 years ago

Yeah.. The documentation pretty much suggests going the (documented) manual install way if you already have something there so I think it would suffice for the script to be like "Ok you already have a pre-commit hook installed, please install manually according to instructions at <url>"