4
\$\begingroup\$

I wrote this script to automatically install the relevant version of MacPorts. It's supposed to be used in a fashion similar to the official installer for HomeBrew.

FYI: The real reason that I decided to write this script is; I'm not all that confident in every aspect of my bash scripting abilities and I needed a project to practice on.

While I have a decent working knowledge of bash from an REPL standpoint, I haven't been scripting very long; so keep that in mind.


macports-installer.sh

#!/bin/bash
#
urisvn=https://svn.macports.org/repository/macports/trunk/www/includes/common.inc
uridist=https://distfiles.macports.org/MacPorts
#
mpvers=$(curl -s $urisvn | grep -m 1 -e \'*.*.*\' | cut -d \' -f 2)
osmajor=$(uname -r | cut -d . -f 1)
#
case $osmajor in
8)
    osvers=10.4
    osname=Tiger
    ;;
9)
    osvers=10.5
    osname=Leopard
    ;;
10)
    osvers=10.6
    osname=SnowLeopard
    ;;
11)
    osvers=10.7
    osname=Lion
    ;;
12)
    osvers=10.8
    osname=MountainLion
    ;;
13)
    osvers=10.9
    osname=Mavericks
    ;;
14)
    osvers=10.10
    osname=Yosemite
    ;;
15)
    osvers=10.11
    osname=ElCapitan
    ;;
*)
    echo Warning: unknown OS version
    ;;
esac
#
mppkg=MacPorts-${mpvers}-${osvers}-${osname}
mpchk=MacPorts-${mpvers}
echo $mppkg.pkg
echo $mpchk.chk.txt
curl $uridist/$mppkg.pkg -O
curl $uridist/$mpchk.chk.txt -O
sha256=$(shasum -a 256 ./$mppkg.pkg)
#
if grep $sha256 $mpchk.chk.txt;
    then sudo installer -verboseR -allowUntrusted -pkg ./$mppkg.pkg -target /;
fi
#
rm ./$mppkg.pkg
rm ./$mpchk.chk.txt
#
if ! grep -F 'export PATH="/opt/local/bin:/opt/local/sbin:$PATH"' .bash_profile;
    then echo 'export PATH="/opt/local/bin:/opt/local/sbin:$PATH"' >> .bash_profile;
fi
#

Also, don't forget to mention if there's anything good about the script. If there's anything you do like about it; or if it does something well, let me know..

\$\endgroup\$
  • \$\begingroup\$ Do not pass -allowUntrusted to sudo installer. That creates a security hole and is completely unnecessary. The MacPorts installers are signed and do not require this flag. \$\endgroup\$ – neverpanic May 12 '16 at 5:51
  • \$\begingroup\$ @neverpanic Could you elaborate? Or demonstrate an appropriate alternative? Thanks for the feedback. Much appreciated. \$\endgroup\$ – voices May 12 '16 at 15:11
  • \$\begingroup\$ This option allows installation of packages signed with untrusted or expired certificates. This is not required because the MacPorts installers are signed with valid Apple Developer ID certificates. Disabling it disables an additional protection layer that would prevent tampering with the installer pkg. \$\endgroup\$ – neverpanic May 16 '16 at 16:09
  • \$\begingroup\$ @neverpanic I see. Back then I must have had trouble getting it to install for some reason. How could it be tampered with though? I'm trying to imagine a scenario where that might actually happen. \$\endgroup\$ – voices May 17 '16 at 10:37
4
\$\begingroup\$

You've got 0 validation in there, so the slightest hiccup will halt your script halfway. That's fine for you, but for a user, there will be a script that runs, crashes along the way, and then leaves all sorts of junk lying about.

You also have a "warning" for an unknown OS version, but then the os version seems to be required - without it, mppkg won't make sense and it's likely that the download will fail (unless there is a standard file, MacPorts-someversion-.pkg?). You'd be better off exiting the script then and there. Fail fast.

Lastly, the two paired curl calls

curl $uridist/$mppkg.pkg -O
curl $uridist/$mpchk.chk.txt -O

Could be ran asynchronously (see this serverfault question https://serverfault.com/questions/456490/execute-curl-requests-in-parallel-in-bash) with & and wait. That way the script would be a bit faster.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Thanks for your help with this one. Can you please explain "validation" and "fail fast"? \$\endgroup\$ – voices May 17 '16 at 10:43
  • 1
    \$\begingroup\$ "validation" = checks if things went okay. "fail fast" = to halt at the first error, to prevent further damage (like if you failed to get the temp directory created or something because it already existed - halting is preferred to deleting the directory in the cleanup phase, destroying the user's data) \$\endgroup\$ – Pimgd May 18 '16 at 8:39
2
\$\begingroup\$

Current directory assumption

You are assuming that the script is executed in the correct user's home directory. If not, then you might be writing to inappropriate .bash_profile file.

It's good practice to write temporary files to $TMPDIR rather than to the current directory.

Error handling

If an error occurs (e.g. "unknown OS version" or download failure), the script blithely attempts to continue.

If a checksum failure occurs, then it exits silently with status code 0.

OS version switch

Instead of inferring the OS version based on the Darwin kernel release number, it would be simpler to read the version of Mac OS using sw_vers -productVersion.

Note that for versions ≤ 10.5, the MacPorts installer is distributed as a .dmg rather than a .pkg. You probably shouldn't try to support those ancient releases.

I am dissatisfied with the entire strategy for determining the URL from which to download the package:

  1. You download common.inc, which contains PHP code.
  2. Then, you "parse" the PHP using Bash to extract the MacPorts release number (mpvers).
  3. To construct the package URL, you reproduce the logic of common.inc in Bash. In particular, you hard-code strings like uridist=https://distfiles.macports.org/MacPorts and mppkg=MacPorts-${mpvers}-${osvers}-${osname}.

In my opinion, this is a fragile approach. Why not use PHP to interpret common.inc? Mac OS comes installed with a PHP interpreter.

To be fair, executing common.inc as PHP code also carries a risk: we would be making an assumption that it has no nasty side-effects. By convention, the .inc extension implies that it should be side-effect free. In general, executing network code is a bad idea, but it's mitigated by the fact that it is served over HTTPS, and that you intend to trust the MacPorts developers anyway.

Code organization

For scripts of this complexity, it is a good idea to define functions. You would be better able to see the big picture of what the code does without wading through the whole thing from top to bottom. I've done it in my PHP solution below, but the same principle would apply to Bash.

Suggested solution

Consider this alternate implementation in PHP. It's longer than yours, but much of the excess is due to error handling, which your script lacked.

#!/usr/bin/php -d allow_url_include=1 -d error_reporting=~EWARNING
<?php

# Need to set allow_url_include=1 to allow this include over the network.
# Need to set error_reporting=~EWARNING to suppress a warning when common.inc
# tries to read a non-existant file /var/www/script_data.

require 'https://svn.macports.org/repository/macports/trunk/www/includes/common.inc';

function pkgUrl() {
    $osVersion = `sw_vers -productVersion`;
    switch(preg_replace('/\.\d+\s*$/', '', $osVersion)) {
        # Uses globals from common.inc
        case '10.6':  return $GLOBALS['snowleopard_pkg'];
        case '10.7':  return $GLOBALS['lion_pkg'];
        case '10.9':  return $GLOBALS['mavericks_pkg'];
        case '10.10': return $GLOBALS['yosemite_pkg'];
        case '10.11': return $GLOBALS['elcapitan_pkg'];
        default:      throw new Exception("Unsupported OS version $osVersion");
    }
}

function download($url) {
    $filename = $_ENV['TMPDIR'] . '/' . basename(parse_url($url, PHP_URL_PATH));
    if (FALSE === file_put_contents($filename, file_get_contents($url))) {
        throw new Exception("Failed to download $url");
    }
    return $filename;
}

function verifyChecksum($filename) {
    # Uses $checksums from common.inc
    $correctChecksums = file_get_contents($GLOBALS['checksums']);
    $sha256 = hash_file('sha256', $filename);
    if (FALSE === strstr($correctChecksums, $sha256)) {
        throw new Exception("Incorrect checksum for $filename");
    }
}

function appendIfNoSuchLine($filename, $line) {
    if (FALSE !== strstr($line . "\n", file_get_contents($filename))) {
        return;
    }
    if (!($fp = fopen($filename, 'a'))) {
        throw new Exception("Could not write to $filename");
    }
    fwrite($fp, $line . "\n");
    fclose($fp);
}

try {
    $pkg = download(pkgUrl());
    verifyChecksum($pkg);
    system(
        sprintf('sudo installer -verboseR -allowUntrusted -pkg %s -target /', escapeshellarg($pkg)),
        $status
    );
    if (0 != $status) {
        throw new Exception("Installation failure");
    }
    unlink($filename);
    appendIfNoSuchLine(
        $_ENV['HOME'] . '/.bash_profile',
        'export PATH="/opt/local/bin:/opt/local/sbin:$PATH"'
    );
} catch (Exception $e) {
    echo $e->getMessage(), "\n";
    return 1;
}
|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ I don't know anything about php. Like I said; the whole idea was to practice bash scripting. Do you really consider this script to be complex, by the way? Also, i don't know what you mean by common.inc. I appreciate your input; some parts are a bit unclear, though. (or presume prior knowledge of certain concepts). Maybe you could demonstrate with actual example snippets of your suggestions? Thanks again. \$\endgroup\$ – voices May 12 '16 at 15:04
  • \$\begingroup\$ What I posted is a complete working solution. You can just save it, make it executable (chmod +x), and run it the same way as you would run your existing script. \$\endgroup\$ – 200_success May 12 '16 at 20:10
  • \$\begingroup\$ I see that. I'm referring to your bash-related comments, only though. That's currently where my interest / focus lies. \$\endgroup\$ – voices May 13 '16 at 12:12
  • \$\begingroup\$ Also, could you please demonstrate how; I could incorporate "error handling" into my script? \$\endgroup\$ – voices May 17 '16 at 11:02
  • \$\begingroup\$ ||exit; ? \$\endgroup\$ – voices May 25 '16 at 18:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.