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

Vagrant fix: pass util_dir in using env in shell provisioner, otherwise use dirname #4842

Closed
wants to merge 2 commits into from
Closed

Conversation

ghost
Copy link

@ghost ghost commented Jan 14, 2019

Description

I made these changes to get Vagrant working for me (in this case with Virtualbox). It should just impact how qmk_install.sh works when it's being run from Vagrant (util_dir will be passed in as /vagrant/util, where the other shell scripts live on the Vagrant box) and otherwise will use dirname. I also had to add -y to apt-get, otherwise it aborted; and I removed the -update argument to qmk_install.sh because from what I could tell it wasn't actually being used anywhere (maybe I missed something).

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

I didn't find any open issues related to this, but I was getting the following output when running VAGRANT_LOG=debug vagrant up:

 INFO ssh: Execute: chown -R vagrant /tmp/vagrant-shell (sudo=true)
DEBUG ssh: stderr: stdin: is not a tty

DEBUG ssh: stderr: 41e57d38-b4f7-4e46-9c38-13873d338b86-vagrant-ssh
DEBUG ssh: Exit status: 0
DEBUG ssh: Uploading: /var/folders/g8/4hmnsljd2jv_pgww4ggts8q00000gn/T/vagrant-shell20190113-66117-e91381.sh to /tmp/vagrant-shell
DEBUG ssh: Re-using SSH connection.
 INFO interface: detail: Running: /var/folders/g8/4hmnsljd2jv_pgww4ggts8q00000gn/T/vagrant-shell20190113-66117-e91381.sh
 INFO interface: detail:     default: Running: /var/folders/g8/4hmnsljd2jv_pgww4ggts8q00000gn/T/vagrant-shell20190113-66117-e91381.sh
    default: Running: /var/folders/g8/4hmnsljd2jv_pgww4ggts8q00000gn/T/vagrant-shell20190113-66117-e91381.sh
DEBUG ssh: Re-using SSH connection.
 INFO ssh: Execute: chmod +x '/tmp/vagrant-shell' && /tmp/vagrant-shell -update (sudo=true)
DEBUG ssh: stderr: stdin: is not a tty

DEBUG ssh: stderr: 41e57d38-b4f7-4e46-9c38-13873d338b86-vagrant-ssh
DEBUG ssh: stderr: /tmp/vagrant-shell: 17: exec:
 INFO interface: detail: /tmp/vagrant-shell: 17: exec:
 INFO interface: detail:     default: /tmp/vagrant-shell: 17: exec:
    default: /tmp/vagrant-shell: 17: exec:
DEBUG ssh: stderr: /tmp/linux_install.sh: not found
 INFO interface: detail: /tmp/linux_install.sh: not found
 INFO interface: detail:     default: /tmp/linux_install.sh: not found
    default: /tmp/linux_install.sh: not found
DEBUG ssh: stderr:

DEBUG ssh: Exit status: 127
ERROR warden: Error occurred: The SSH command responded with a non-zero exit status. Vagrant
assumes that this means the command failed. The output for this command
should be in the log above. Please read the output to determine what
went wrong.

It looked like this was because Vagrant uploads qmk_install.sh to /tmp/vagrant-shell on the VM, so the script was setting util_dir to /tmp instead of the directory where the scripts actually live on the VM (/vagrant/util).

The issue with apt-get was this:

    default: Need to get 74.9 MB of archives.
    default: After this operation, 353 MB of additional disk space will be used.
    default: Do you want to continue?
    default:  [Y/n]
    default: Abort.

The -y flag fixed this. There's still an error for a missing signature on a puppetlabs.com repo when apt-get update runs, but it wasn't fatal so I didn't really look into it further. For reference:

    default: W: An error occurred during the signature verification. The repository is not updated and the previous index files will be used. GPG error: http:
//apt.puppetlabs.com trusty Release: The following signatures couldn't be verified because the public key is not available: NO_PUBKEY 7F438280EF8D349F
    default:
    default: W: Failed to fetch http://apt.puppetlabs.com/dists/trusty/Release
    default:
    default: W: Some index files failed to download. They have been ignored, or old ones used instead.

Checklist:

  • My code follows the code style of this project.
    • I noticed the code style says that indentation should use two spaces, but qmk_install.sh had tabs so I just stuck with that for my small addition.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document. (https://docs.qmk.fm/#/contributing)
  • 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).

@zvecr
Copy link
Member

zvecr commented Jan 14, 2019

Its good to see someone else is seeing the same issues as me (and is also working on vagrant).

My change to the Vagrantfile was:

  # This script ensures the required packages for AVR programming are installed
  # It also ensures the system always gets the latest updates when powered on
  # If this causes issues you can run a 'vagrant destroy' and then
  # add a # before ,run: (or change "always" to "once") and run 'vagrant up' to get a working
  # non-updated box and then attempt to troubleshoot or open a Github issue
  config.vm.provision "shell", inline: "/vagrant/util/qmk_install.sh", run: "always"

this didn't require any other changes to the qmk_install.sh script as well as attempting to clarify the above comment about the vagrant update logic.

As for the change to linux_install.sh, it seems to be by design that the various install options prompt for confirmation, rather than assume yes. The Ubuntu path being changed deviates away from the others. Should the required change check an environment variable or script argument is set, and only then pass the -y argument? and should this be done for all install targets?

@zvecr
Copy link
Member

zvecr commented Jan 16, 2019

Just in case it helps, i have pushed my version of changes here.

To help give a little context, so far i have:

  • Updated base image to one that is newer, and more maintained while still supported the existing providers
  • Provided a name to vagrant instead of it using 'default'
  • Fixed install issues in a slightly different way
    • Instead of changes to linux_install.sh described and used above, i just pipe to yes to localise the changes to just the Vagrantfile
    • Use the full path and inline to resolve the copy to /tmp issue
  • Provided an example to fix clock skew issues on virtualbox (this could be enabled by default)

I hope to open a pull for at least some of the other bits and pieces.

@drashna
Copy link
Member

drashna commented Jan 25, 2019

Looks like there is a merge conflict now!

@zvecr
Copy link
Member

zvecr commented Jan 25, 2019

@drashna #4900 has been merged, which tackled the same issues. This change could be refactored for a belt and braces approach, but largely surplus to requirements.

@drashna
Copy link
Member

drashna commented Jan 25, 2019

Okay.

In that case, I'll close this.

If needed, please reopen it then.

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