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 dependencies and scripts for MacOS and Homebrew. #15

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

todd-a-jacobs
Copy link

  • 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
Copy link

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.

Comment on lines +24 to +27
which zbarimg > /dev/null 2>&1 || {
echo "zbarimg not found in PATH" > /dev/stderr
exit 2
fi
}
Copy link

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
Copy link

Choose a reason for hiding this comment

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

Suggested change
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
Copy link

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.

Comment on lines 36 to 38
| 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'
Copy link

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:

Suggested change
| 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...

Comment on lines +1 to +18
#!/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
Copy link

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?

Comment on lines +226 to +235
[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.*
Copy link

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:

Suggested change
[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 -->

Copy link

@anarcat anarcat left a 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.

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.

2 participants