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

chore(install): Add Arch Linux installation #1251

Merged
merged 5 commits into from
Apr 15, 2019

Conversation

coredump
Copy link
Contributor

Not a lot of changes, since bash is bash anywhere, but this is a list:

  • Uses who instead of who -m that doesn't behave correctly on arch,
    I guess something different about how loginctl works, but couldn't
    find anything specific.
  • Uses pacman to install the needed java/ca packages instead of dpkg.
  • Removed the migration check, that is specific to past debian
    packages.
  • Uses a sysusers file to create system groups instead of using
    groupadd as suggested by the packaging guidelines.

Still mentions debian for the download location/archives since there's no
generic named file to download.

Tested on Arch and Manjaro, and worked fine. There may be some
imcompatibility with other arch based distros but should cover most of
the options around.

Not a lot of changes, since bash is bash anywhere, but this is a list:

 * Uses `who` instead of `who -m` that doesn't behave correctly on arch,
 I guess something different about how loginctl works, but couldn't
 find anything specific.
 * Uses pacman to install the needed java/ca packages instead of dpkg.
 * Removed the migration check, that is specific to past debian
 packages.
 * Uses a `sysusers` file to create system groups instead of using
 `groupadd` as suggested by the packaging guidelines.

 Tested on Arch and Manjaro, and worked fine. There may be some
 imcompatibility with other arch based distros but should cover most of
 the options around.
@spinnakerbot
Copy link
Contributor

The following commits need their title changed:

  • 3a8a2d5: Adapted install script for arch based distros

Please format your commit title into the form:

<type>(<scope>): <subject>, e.g. fix(kubernetes): address NPE in status check

This allows us to easily generate changelogs & determine semantic version numbers when cutting releases. You can read more about commit conventions here.

Not a lot of changes, since bash is bash anywhere, but this is a list:

 * Uses who instead of who -m that doesn't behave correctly on arch,
 I guess something different about how loginctl works, but couldn't
 find anything specific.
 * Uses pacman to install the needed java/ca packages instead of dpkg.
 * Removed the migration check that was specific to past debian
 packages.
 * Uses a sysusers file to create system groups instead of using
 groupadd as suggested by the packaging guidelines.

Still mentions debian for the download location/archives since there's no
generic named file to download.

Tested on Arch and Manjaro, and worked fine. There may be some
imcompatibility with other arch based distros but should cover most of
the options around.
@coredump coredump changed the title Adapted install script for arch based distros chore(install): Add Arch Linux installation Mar 19, 2019
Copy link
Contributor

@ezimanyi ezimanyi left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! This is similar enough to the debian install script that I almost want to see if there's a way to just have conditional logic in the script that branches based on the install (not that it's great to have too much logic in bash scripts...)

Some notes on the differences:

  • who vs. who -m: I left a comment that whoami might just work for both
  • pacman: This could just be another if block in install_java (which already branches on distribution)
  • migration check: It looks like that's just a no-op if dpkg is missing, so it might be safe to just let this run anyway for arch linux
  • sysusers vs. groupadd: This could probably just be another branch on the distro ID

Let me know your thoughts---my main concern with having this be a completely separate script is that people will forget to update it when they either add features or make Halyard changes that require a change in the install script. So I'd lean a bit towards branching in the same script for that reason, even though it does add a bit of complexity (particularly for a bash script).

install/arch/InstallHalyard.sh Outdated Show resolved Hide resolved
This makes the old InstallHalyard less debian/ubuntu centric by adding
Arch Linux based distros to it. Moved it to a `linux` directory,
following the current standard (there's a `macos` directory)

Tested on pristine vagrant images and both install correctly. Should be
easy to adapt to other distros.
@coredump
Copy link
Contributor Author

Modified the PR to make it a single script, as @ezimanyi suggested.

Copy link
Contributor

@ezimanyi ezimanyi left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@dansimau
Copy link

@ezimanyi
Copy link
Contributor

Whoops, I didn't notice in the PR diff that the file was moved as well as changed. I think we should leave this at the original path...while it would be technically more correct to have it in a path called linux rather than debian this change has the potential to break automation that people have set up and I'm not sure being a bit more correct is worth that breakage.

@ezimanyi
Copy link
Contributor

Moving the script back in #1277.

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.

4 participants