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

Track downloaded base boxes URLs into a state file and allow hooking into base boxes removal #2327

Merged
merged 13 commits into from
Nov 25, 2013

Conversation

fgrehm
Copy link
Contributor

@fgrehm fgrehm commented Oct 5, 2013

I've been willing to do this for a while and last night I was bitten by an issue that would be easily diagnosed if I knew from where the box was downloaded. Basically the URL written on the Vagrantfile was different from the one I downloaded but both boxes had the same name :(

The newly added StateFile class can be improved but as usual, I think this is a good start :)

Reference: #2293
/cc @mgedmin

@mitchellh
Copy link
Contributor

I like the general idea. I think instead of a dedicated state file, we should just put a state file into each box directory. This will be easier to keep track of because then the whole directory can just be removed when removing the box.

I think this should target Vagrant 1.4 due to the magnitude of the change.

I'm not convinced yet of the "-i" option when listing boxes. I think maybe we should just show it as part of vagrant box list. I want to get machine-readable into 1.4 so if people are trying to script Vagrant they can just get machine readable output out of vagrant box list

@fgrehm
Copy link
Contributor Author

fgrehm commented Oct 5, 2013

Cool, I'll update the PR accordingly :)

Regarding the "-i" option, I just think that if we are going to always show that information we might need to change the output as it is very verbose in my option. If you can think of a better way to present that just LMK :)

Just to double check, what do you think about adding those new actions / middlewares? Am I on the right track?

As a note to myself, we'll need to make sure the dedicated state file gets skipped when repackaging boxes with vagrant package as it won't make sense to include them on the .box package. I'm not sure if Vagrant automatically packages the whole dir or not but might be worth checking it out.

@fgrehm
Copy link
Contributor Author

fgrehm commented Oct 5, 2013

Oh, and I agree to target 1.4 and I have just tagged this on the 1.4 GitHub milestone ;)

@fgrehm
Copy link
Contributor Author

fgrehm commented Oct 18, 2013

@mitchellh I've updated the branch with the changes we discussed over IRC but I kept the -i option for now. Please LMK if you think I should just drop it or if you think we can change the output to be less verbose in some way :)

I don't really like the extra_info on method names but unfortunately I'm out of ideas on how to name those methods. If you have a better idea just LMK

@mitchellh
Copy link
Contributor

Thanks @fgrehm. I'm going to get this in today. I'm going to make some minor changes but the core functionality is great.

@fgrehm
Copy link
Contributor Author

fgrehm commented Nov 25, 2013

Awesome :)

@mitchellh mitchellh merged commit 4887512 into hashicorp:master Nov 25, 2013
@mitchellh
Copy link
Contributor

Merged!

@fgrehm
Copy link
Contributor Author

fgrehm commented Nov 25, 2013

🎆 awesome! 🎆

@fgrehm fgrehm deleted the 2293-track-box-url branch November 25, 2013 21:31
@ghost ghost locked and limited conversation to collaborators Apr 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants