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

Install script is non-standard, incomplete, hard to follow and Linux distribution dependent #4

Open
JayFoxRox opened this issue Apr 19, 2018 · 0 comments

Comments

@JayFoxRox
Copy link

JayFoxRox commented Apr 19, 2018

The install script has a handful of issues.

First, let's split it into seperate lines and add a shebang for some sanity:

#!/usr/bin/env bash

# Install dependencies
sudo apt-get install libudev-dev libusb-1.0-0-dev libfox-1.6-dev
sudo apt-get install autotools-dev autoconf automake libtool

# Install first part of SwitchProCon
sudo cp 99-hid.rules /etc/udev/rules.d/99-hid-procon.rules

# Compile hidapi
mkdir hidapi
cd hidapi
git clone git://github.com/signal11/hidapi.git .
./bootstrap
./configure
make
sudo make install
cd ..
rm -rf hidapi

# Build SwitchProCon
mkdir build
cd build
cmake ..
make

# Install SwitchProCon ?!?!

Some issues:

  • No filename extension which suggests what it is, consider renaming to install.sh
  • The script should use pushd and popd if necessary
  • SwitchProCon is never installed, but the udev-rules are
  • The udev rules are even installed when compilation fails
  • The script uses sudo internally
  • The script depends on apt-get which is not found in a lot of popular distributions
  • The install script installs part of the toolchain (but somehow expects git, cmake, gcc, ..)
  • hidapi is compiled from source, without any reason being mentioned anywhere, however, at least Ubuntu and Arch Linux ship a version of hidapi on their repos

So to resolve some issues I'd recommend:

  • Seperate building and installation (for example, through cmake) to avoid having to use sudo before the build even starts
    • Get rid of the install script
    • Put the HID rule installation into the cmake install step
  • If you need a non-stable hidapi, put it into a git submodule and launch compilation from cmake
  • List dependencies so users can install them on their own (or package your driver for their distribution themselves)
  • For development builds, expect that the user already has a toolchain installed and optionally list what they'd need to have it
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

No branches or pull requests

1 participant