Update to vagrant 2.1.1
ClosedPublic
Actions

Authored by italy-portugal on Jun 5 2018, 2:59 AM.

Details

Reviewers

• sunnyflunk

Group Reviewers

Triage Team

Commits

R3106:4c5cd83e4e1a: Update to vagrant 2.1.1

Summary

core: Vagrant aliases [GH-9504]
core: Integrate vagrant-triggers plugin functionality into core Vagrant [GH-9713]

Test Plan

Install locally and confirmed version
Run with a few local projects, including usage of vagrant up destroy init and provision

Diff Detail

Repository

R3106 vagrant

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

italy-portugal requested review of this revision.Jun 5 2018, 2:59 AM

italy-portugal created this revision.

Note that this patch removes the abi_used_libs file, which is an essential file. So either you have manually removed it or don't have abireport set up correctly. Either way it needs to be resolved and it should be regenerated each time you run make.

So I have merged the deps for this as I knew exactly what you were doing (even though it would have been rather unclear to others), but I do have some feedback that will streamline and make your patches better:

Use capital letters for commit entries when using a sentence (i.e. Initial commit of..)
For some reason the %gem_install line on some of the packages was only 1 space. All the spacings should be 4 spaces.
All patches require test plans, in these cases they were tested with the vagrant update (which I knew), but it's usually best to include that in their commits also (i.e. pretend we aren't familiar with what you've done).
Phab has some great notation you can use that helps us greatly. Typically you would push your patches in the order they would be built. i.e. ruby-ffi-win32-extensions first, then ruby-win32-file-stat etc. What this allows, is then in the commit for ruby-win32-file-stat you can write 'Depends on D3037' and Phab will link them all together. This basically tells us, that D3037 needs to be pushed first. Doing this for all commits links the chain back to vagrant. So this commit would be 'Depends on D3038 and D3039'

This revision now requires changes to proceed.Jun 6 2018, 12:41 AM

Thanks for the feedback!

I can't seem to get abireport to give the expected output for this package. I've gotten the correct output from running make on other packages, but for vagrant it outputs an empty file, both for this submission and what is currently on master. Are there any pointers that you could give for getting abireport set up correctly for this package?

Well when running make it does two steps. Builds the package, then runs make abireport. It requires that the built package is in the directory.

So after building the package (ensuring the eopkg file is present), run make abireport and output the full log of that command.

Note I'm going to fix this, but you will need to sort out your environment, as we don't want to lose abi files.

This revision is now accepted and ready to land.Jun 14 2018, 3:45 AM

Closed by commit R3106:4c5cd83e4e1a: Update to vagrant 2.1.1 (authored by italy-portugal, committed by • sunnyflunk). · Explain WhyJun 14 2018, 3:54 AM

This revision was automatically updated to reflect the committed changes.

Ok, turns out that your build environment is likely to be fine, simply the previous commit was incorrect (which was mine when I split out all ruby packages xD).

Thanks for the patches

That's a relief, I was working on a forum post to ask about what I need to fix.

Sorry for not responding sooner, I've been without internet for a few days

der_eismann mentioned this in T6437: Update vagrant 2.1.1 (or later).Jul 13 2018, 7:25 PM