Improvements to install_compiler.sh and install_tytools.sh - #208
Open
roccoagain wants to merge 4 commits into
Open
Improvements to install_compiler.sh and install_tytools.sh#208roccoagain wants to merge 4 commits into
roccoagain wants to merge 4 commits into
Conversation
The installer removed tools/compiler before attempting the download, so a failed run (no network, Arm CDN down) left the machine with no toolchain and no way to get one back. Nothing checked exit status either, so a failed download fell through to tar and mv errors and still exited 0, letting `make install` report success. Restructure to download, verify, then swap: - Download to a staging dir inside tools/compiler and only move the old toolchain aside once the new one has extracted cleanly. An EXIT trap restores it if any step in between fails. - Verify the tarball against a pinned SHA256 per platform before extracting. - set -euo pipefail, plus an explicit error and non-zero exit for every unsupported OS/arch instead of silently falling through. - Use curl rather than wget, dropping the Homebrew dependency on macOS since curl ships with the system. Resume on retry instead of restarting the transfer, and keep the progress meter off when stdout is not a TTY. - Collapse the four URL/checksum variables and nested case blocks into one platform table so a version bump is a single contiguous edit. - Normalise uname -m under Rosetta so an Apple Silicon Mac in a translated shell installs the native arm64 toolchain. - Add the missing shebang and derive paths from BASH_SOURCE, so the script works when run directly or from another directory. install_arduino.sh invokes it both ways. Drop the tools/tools gitignore entry, which only existed because the old script wrote to cwd-relative paths. Verified on macOS arm64 and, with a stubbed uname, both Linux targets: checksums match and the extracted binaries are the expected ELFs. Failure paths (404, checksum mismatch, unsupported platform, interrupted swap, missing xz) all exit non-zero and leave the existing toolchain intact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The macOS branch cloned CU-Robotics/tycmd at whatever main pointed to and moved the prebuilt binary into /usr/local/bin with sudo. Nothing pinned the commit or verified the contents, so an accidental push to that repo changed what every teammate installed as root. - Pin the binary to a commit and check its SHA256 before installing. The pinned bytes are identical to what the previous code installed, so nobody's setup changes. Fetching the file directly rather than cloning is what makes pinning possible, and drops the git dependency on macOS as a side effect. - Create /usr/local/bin before installing into it. macOS does not create it, and Homebrew on Apple Silicon uses /opt/homebrew, so a clean Mac can lack it and the old `sudo mv` failed with "No such file or directory". - Use `install -m 0755` instead of `mv` followed by `chmod`. - Clean up the temp directories with an EXIT trap in both branches. Both removed them on the last line, so any failure in between leaked one, and the "binary not found" exit leaked one unconditionally. The Arch build now runs in a subshell, so the `cd -` before the cleanup is gone too. Verified on macOS with sudo stubbed and the destination redirected: success, checksum mismatch, and a bad pinned commit all behave, and the two failures leave an existing binary untouched. Temp-dir leaks measured against the old code as a control: it leaked one directory on a failed build, this does not. The Arch and Debian branches were only exercised with stubbed pacman, git, makepkg and apt, so the real AUR build and repo setup are still untested. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Installing dependencies should not permanently change a machine's package sources. The Debian branch added download.koromix.dev to sources.list.d and its signing key to /etc/apt/keyrings, so `make install` left the host trusting a third-party repo for every future apt upgrade. Download the tytools .deb directly from the same host, verify its SHA256, and hand it to apt. Nothing persists in /etc/apt, and the whole team runs an identical build rather than whatever the repo has published since. apt still resolves the Qt6 and udev dependencies from the distro's own repositories, so only the tytools package itself comes from upstream. The pinned version and per-architecture checksums come from the repo's own Packages index, and the comment records where to find them when bumping. Also error clearly on architectures upstream does not build. The package exists for amd64 and arm64 only, so armhf previously failed with an apt dependency error rather than an explanation. Note that the .deb depends on libqt6core6, which Ubuntu ships from 22.04 onwards. On a Jetson this works on JetPack 6 but not on 4 or 5, which is unchanged by this commit but worth recording. Verified with sudo, apt and dpkg stubbed: the real .deb downloads and its checksum matches for both amd64 and arm64, an unsupported architecture exits with a useful message, and a checksum mismatch installs nothing and leaves no temporary directory behind. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tools/install_arduino.sh installed arduino-cli and the teensy:avr core, but nothing invokes it: 'make install' runs install_tytools.sh and install_compiler.sh only, and no other script, README, or docs page references it. Nothing in the build needs what it installed either. The Makefile compiles against the vendored teensy4/ sources with the arm-none-eabi toolchain from install_compiler.sh, so the Teensyduino core it fetched went unused, as did the ~/.arduino15 and ~/Library/Arduino15 path detection that was dropped from the Makefile separately. It called install_compiler.sh at the end, so it was the dependent rather than a dependency; removing it leaves the installers reachable through 'make install' unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
roccoagain
changed the title
|
Doxygen warnings output: (if empty, there are no warnings). Please correct any warnings before merging. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Some refinements to our install scripts:
install_compiler.shnow downloads and verifies the new compiler before touching the existing one. The old script deletedtools/compilerup front, so a dropped connection or a Ctrl-C left you with no compiler and no way to get one back..deb, and the macOStycmdbinary.set -euo pipefail. Nothing checked exit status before, so a failed download fell through totar/mverrors and still exited 0 —make installwould report success on a broken install..debinstead of adding a repo to APT. Every install gets the sametycmd, and nothing persists in/etc/apt.tycmdis pinned to a commit, rather than cloning whatever is onmainandsudo mv-ing it into your PATH.install_arduino.sh. It installed arduino-cli and theteensy:avrcore, but we vendor the Teensy core inteensy4/and install the ARM toolchain ourselves.Tested with
make installon macOS. Worth testing on Debian and Arch before merging.