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

Refactor qmk_install.sh #10681

Merged
merged 10 commits into from
Nov 26, 2020
Merged

Refactor qmk_install.sh #10681

merged 10 commits into from
Nov 26, 2020

Conversation

fauxpark
Copy link
Member

@fauxpark fauxpark commented Oct 18, 2020

Description

Splits each platform/distro out into its own file, which qmk_install.sh simply sources before running _qmk_install_prepare and (if successful) then _qmk_install.

Will need a bit of testing...

  • Arch/Manjaro
  • Debian/Ubuntu
  • Fedora
  • FreeBSD
  • Gentoo (won't test, but probably works)
  • macOS (Homebrew)
  • OpenSUSE (Leap 15.2/Tumbleweed) (errors with packages, see below)
  • Sabayon (error with crossdev, see below)
  • Slackware (won't test, but probably works)
  • Solus (works, but requires reopening terminal)
  • Void
  • Windows (MSYS2)

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@fauxpark fauxpark force-pushed the qmk-install-refactor branch from b6cd5b9 to 921db94 Compare October 18, 2020 11:13
Comment on lines +1 to +31
#!/bin/bash

_qmk_install_prepare() {
case $(grep ID /etc/os-release) in
*15.1*)
REPO_RELEASE=Leap_15.1;;
*15.2*)
REPO_RELEASE=Leap_15.2;;
*)
#REPO_RELEASE=Tumbleweed;;
echo "ERROR: Tumbleweed is currently not supported."
exit 1
esac

sudo zypper addrepo https://download.opensuse.org/repositories/devel:gcc/openSUSE_$REPO_RELEASE/devel:gcc.repo
sudo zypper addrepo https://download.opensuse.org/repositories/hardware/openSUSE_$REPO_RELEASE/hardware.repo
sudo zypper --gpg-auto-import-keys refresh
}

_qmk_install() {
echo "Installing dependencies"

sudo zypper install -y \
make clang gcc unzip wget zip \
python3-pip \
cross-avr-binutils cross-avr-gcc8 avr-libc \
cross-arm-binutils cross-arm-none-gcc8 cross-arm-none-newlib-devel \
avrdude dfu-programmer dfu-util

python3 -m pip install --user -r $QMK_FIRMWARE_DIR/requirements.txt
}
Copy link
Member Author

Choose a reason for hiding this comment

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

There is no Tumbleweed directory for devel:gcc, and cross-arm-none-gcc8 throws a bunch of linker errors on Leap... so OpenSUSE support is broken at this time. If any OpenSUSE user is able to provide a fix (or better yet, fix the packages and repos), it would be much appreciated!

Copy link
Member

Choose a reason for hiding this comment

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

Would it be worth grabbing the official ARM toolchain in this circumstance, then?
https://developer.arm.com/tools-and-software/open-source-software/developer-tools/gnu-toolchain/gnu-rm/downloads

Copy link
Member Author

@fauxpark fauxpark Oct 23, 2020

Choose a reason for hiding this comment

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

I'd kinda like to avoid another ~/qmk_utils folder if at all possible, considering you have to modify the .bashrc to add it to PATH (and some users may not be running bash), and then a new terminal must be opened before qmk doctor displays the correct results. Preferably the already existing package should be fixed.

@fauxpark
Copy link
Member Author

fauxpark commented Oct 19, 2020

Testing Solus now - @chax is there a particular reason the AVR and ARM toolchain is not symlinked into /usr/bin? It would be preferable not to have to ask users to source a script in order to compile, it should Just Work. (especially since qmk doctor will check for the presence and versions of the GCCs after qmk_install.sh runs, if installing from the CLI)

@chax
Copy link
Contributor

chax commented Oct 19, 2020

It should "just work" if you open new terminal, since those scripts are automatically sourced on every new terminal session. But for current terminal, if you don't source them manually, it won't work. The reason AVR and ARM toolchain are not in standard PATH is that Solus core team didn't want those to be in core part of the system, but installed in separate (custom) path. Maybe you can source them automatically upon completed installation, therefore you won't need to warn user to do that.

Since gcc is not guaranteed to be installed until after that point
@fauxpark fauxpark force-pushed the qmk-install-refactor branch from 6e049e7 to 5d95abb Compare October 20, 2020 02:28
@fauxpark
Copy link
Member Author

fauxpark commented Oct 20, 2020

The reason AVR and ARM toolchain are not in standard PATH is that Solus core team didn't want those to be in core part of the system, but installed in separate (custom) path.

I can understand having the toolchains installed to a separate place, but my question is why are they not symlinked into /usr/bin? Note that these binaries will not conflict with any others as they are prefixed with the target (eg. avr and arm-none-eabi), so it is perfectly safe to have them there.

Unfortunately sourcing the scripts automatically during _qmk_install does not work, presumably because everything is basically run in a subshell.

@chax
Copy link
Contributor

chax commented Oct 20, 2020

Ahh right, i think you're right, i forgot when you run script as a separate process, sourcing environment variables affect only that process and not the terminal session from which script is run. I can talk with Solus core team about symlinks but i think they have the same stance towards symlinks as if it was just the binaries, but maybe they changed their mind a bit in last two years. We'll try to find some solution here, but for now, user must source them themselves manually.

Comment on lines 1 to 15
#!/bin/bash

_qmk_install() {
echo "Installing dependencies"

sudo equo install \
app-arch/unzip app-arch/zip net-misc/wget sys-devel/clang sys-devel/gcc sys-devel/crossdev \
dev-lang/python \
dev-embedded/avrdude dev-embedded/dfu-programmer app-mobilephone/dfu-util

sudo crossdev -s4 --stable --g \<9 --portage --verbose --target avr
sudo crossdev -s4 --stable --g \<9 --portage --verbose --target arm-none-eabi

python3 -m pip install --user -r $QMK_FIRMWARE_DIR/requirements.txt
}
Copy link
Member Author

Choose a reason for hiding this comment

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

After switching to the sabayonlinux.org repo (rather than sabayon-weekly) I was able to update the distro - for some reason the weekly repo wanted to upgrade mesa to a version that was not in any of the mirrors.
Now, the crossdev commands fail with:

/usr/bin/crossdev: line 1019: multilib_env: command not found

 * error: could not load multilib settings for HCHOST

🤷

@fauxpark
Copy link
Member Author

@petejohanson @csssuf would you mind testing/reviewing the FreeBSD and Gentoo scripts, respectively?

I think in the FreeBSD case, we should not attempt to do anything unless sudo is installed, or let it error out.

util/install/gentoo.sh Outdated Show resolved Hide resolved
@csssuf
Copy link
Contributor

csssuf commented Oct 27, 2020

@fauxpark From a quick read, Gentoo looks good to me other than one small comment - if you want to be extra sure I can give it a proper test later today or tomorrow.

@fauxpark
Copy link
Member Author

@csssuf That'd be helpful, yeah. In particular I suspect the read prompt may not actually pause when qmk_install.sh is run via qmk setup.
It could just be a Windows thing though; I've only so far noticed it in the driver installation prompt for MSYS2, which has since been removed in favour of installing them anyway.

Copy link
Member

@tzarc tzarc left a comment

Choose a reason for hiding this comment

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

Restructuring LGTM, not executed any of them but the concept looks sound.

@fauxpark
Copy link
Member Author

@csssuf did you get time to test this?

And @chax any luck with the Solus stuff?

I think either way, this is probably good to merge as is, and we can work out the kinks for the less popular distros later.

@Erovia Erovia merged commit c21d5a0 into qmk:master Nov 26, 2020
@fauxpark fauxpark deleted the qmk-install-refactor branch November 26, 2020 14:39
xgnxs pushed a commit to xgnxs/qmk_firmware that referenced this pull request Jan 9, 2021
drashna pushed a commit to zsa/qmk_firmware that referenced this pull request Jan 13, 2021
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants