-
Notifications
You must be signed in to change notification settings - Fork 17
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 dependencies and scripts for MacOS and Homebrew. #15
base: master
Are you sure you want to change the base?
Update dependencies and scripts for MacOS and Homebrew. #15
Conversation
todd-a-jacobs
commented
Feb 24, 2022
- Fix scripts to provide better cross-platform support; this should improve the MacOS experience without affecting Linux users in any way.
- Add a setup script to handle dependencies for Homebrew users.
- Document the existence and purpose of the new setup script.
- Add a SPDX identifier for the MIT license.
- Fix the markup and make other minor improvements to the README to hopefully make it more consistent and perhaps slightly clearer.
- Documented a possible point of confusion about the license name in the README.
- Homebrew 3.3.16 (and possibly earlier) on recent versions of macOS are unable to build the necessary Python3 libraries without some additional flags passed to clang and/or llvm. - The Homebrew zbar package is needed to provide the zbarimg executable, but interferes with building zbar-py, so it needs to be removed and re-added after zbar-py is built.
- MacOS and some Linux distros have Bash in a different location, or may not have Bash at all. - Use env to find the location of Bash, rather than hard-coding it. - Some Bashisms in the scripts may prevent it from running with other Bourne-like shells, and definitely won't work with the Fish shell.
- Homebrew typically installs binaries to /usr/local/bin, so prepend that to the PATH. - Alias sed to gnu-sed, or upstream's sed commands will break.
- Possible Bashism. - Prevent history issues when testing at the command line when `set -H` is enabled by placing the negation inside the test brackets.
- Use `which` to check the PATH for zbarimg, rather than hard-coding its location. - Use whichever zbarimg is in the PATH, rather than defining the executable's fully-qualified PATH. - If zbarimg is missing, the error message now identifies the PATH as the problem, rather than pointing to a (possibly) incorrect location for the executable.
Since paperrestore-verify.sh has a dash, the restore script should too.
- Normalize the header structure to avoid jumping from H2 to H5, skipping H3 and H3. - This is mostly cosmetic, but semanticly it improves the HTML.
- Add section for newer MacOS systems since the current list doesn't work as-is on Big Sur with Python 3.9.10 and zbar 0.23.91. - Assumes Homebrew as the package manager, but should provide guidance for anyone using an alternative like MacPorts.
- Added the SPDX identifier for the MIT license. - Add link to the MIT license. - Document the discrepency between the upstream wording and the included license. - Refer questions about the intended license terms to the upstream copyright holder.
- Move links inline where possible. - Minor grammar, spelling, and semantic updates throughout. - Reflow line wrapping to a standard 72 characters. - Sort dependencies. - Indent explicit Python3 dependencies.
@@ -1,33 +1,38 @@ | |||
#!/bin/bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would argue for rewriting this script in plain POSIX instead of trying to guess where the right utilities are.
which zbarimg > /dev/null 2>&1 || { | ||
echo "zbarimg not found in PATH" > /dev/stderr | ||
exit 2 | ||
fi | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i this case i wouldn't bother autodetecting or even warning, you'll have a good error just by calling zbarimg
without a path prefix lower down.
if ! [ -f "$SCANNEDFILE" ]; then | ||
echo "$SCANNEDFILE is not a file" | ||
if [ ! -f "$SCANNEDFILE" ]; then | ||
echo "$SCANNEDFILE is not a file" > /dev/stderr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
echo "$SCANNEDFILE is not a file" > /dev/stderr | |
echo "$SCANNEDFILE is not a file" >&2 |
@@ -1,4 +1,4 @@ | |||
#!/usr/bin/bash | |||
#!/usr/bin/env bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, just make sure the script is posix.
| sed -e "s/\^/\x0/g" \ | ||
| sort -z -n \ | ||
| sed ':a;N;$!ba;s/\n\x0[0-9]* //g;s/\x0[0-9]* //g;s/\n\x0//g' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now this is the tricky bit of course. is there a way this could be ported to BSD sed while still being compatible with gnu sed? maybe it's just a matter of:
| sed -e "s/\^/\x0/g" \ | |
| sort -z -n \ | |
| sed ':a;N;$!ba;s/\n\x0[0-9]* //g;s/\x0[0-9]* //g;s/\n\x0//g' | |
| sed "s/\^/\x0/g" \ | |
| sort -z -n \ | |
| sed ':a;N;$!ba;s/\n\x0[0-9]* //g;s/\x0[0-9]* //g;s/\n\x0//g' |
... it's been a while...
#!/usr/bin/env bash | ||
|
||
set -e | ||
|
||
brew install --cask basictex | ||
brew install \ | ||
enscript ghostscript gnu-sed imagemagick libqrencode pillow python3 | ||
|
||
python3 -m pip install --upgrade pip | ||
python3 -m pip install --upgrade setuptools | ||
|
||
# the homebew zbar package interferes with the compilation of zbar-py | ||
brew uninstall zbar --force | ||
|
||
CFLAGS="-I$(brew --prefix)/include" LDFLAGS="-L$(brew --prefix)/lib" pip3 install qrencode pyx zbar-py | ||
|
||
# (re)install the zbar package to provide the zbarimg binary | ||
brew install zbar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't such a recipe supposed to go upstream in homebrew directly?
[MIT X11 License](https://opensource.org/licenses/MIT) | ||
<!-- SPDX-License-Identifier: MIT --> | ||
|
||
*NB: The upstream author marked this as "MIT X11" (there's an MIT | ||
license, and an X11 license, but there's no SPDX identifier for an "MIT | ||
X11" license) but included the MIT license in the | ||
[LICENSE](https://github.com/intra2net/paperbackup/blob/master/LICENSE) | ||
file. Please contact | ||
[upstream](https://github.com/intra2net/paperbackup) if you're confused | ||
about which license(s) apply.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think upstream is ambiguous at all here actually. there's a LICENSE file right here:
https://github.com/intra2net/paperbackup/blob/master/LICENSE
sure, it says "x11" here, but that's probably an oversight.
it looks like a plain old expat to me, so maybe you'd like to clarify that way instead of introducing more ambiguity. :)
in other words:
[MIT X11 License](https://opensource.org/licenses/MIT) | |
<!-- SPDX-License-Identifier: MIT --> | |
*NB: The upstream author marked this as "MIT X11" (there's an MIT | |
license, and an X11 license, but there's no SPDX identifier for an "MIT | |
X11" license) but included the MIT license in the | |
[LICENSE](https://github.com/intra2net/paperbackup/blob/master/LICENSE) | |
file. Please contact | |
[upstream](https://github.com/intra2net/paperbackup) if you're confused | |
about which license(s) apply.* | |
[MIT "Expat" License](https://opensource.org/licenses/MIT) | |
<!-- SPDX-License-Identifier: MIT --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good ideas all around, but i think there's a handful of tweaks that should be done.