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

config/module: use source as part of key #8398

Merged
merged 1 commit into from
Aug 26, 2016
Merged

Conversation

mitchellh
Copy link
Contributor

Fixes #3070

Ehhh, couldn't find a really great way to test this without embedding a git repo which I didn't want to do here. I view this as sort of a stop-gap solution anyways as we do a bigger plan around how modules should be stored on disk.

This is kind of backwards incompatible in that after updating you'll have to run terraform get again. I don't think that is very disruptive but happy to talk about it.

@jbardin
Copy link
Member

jbardin commented Aug 26, 2016

this LGTM, as long as we're OK with having to re-get.

@mitchellh mitchellh merged commit 4e7f2dd into master Aug 26, 2016
@mitchellh mitchellh deleted the b-module-collide branch August 26, 2016 20:27
@rowleyaj
Copy link
Contributor

@mitchellh, let me know if you'd prefer me to open a new issue rather than comment on this merged PR.

A while ago I looked at changing this in our fork (Ensighten#9), to only use source for the key as it was previously. It looked like it was originally changed away from that (#1418) due to incompatibilities with Windows.

I don't use Windows so can't test, but would this reintroduce the same problems that existed prior to #1418?

I'm interested in improving the module get / update process, but not really sure where to start in the code beyond what I found previously. The reason for wanting to improve this is it takes quite some time for us to download all of our modules, and several are repeated. Whislt testing changing the key back to source helped improve things for us in terms of the initial get, but an update still wants to grab every module. I was wondering if there was a way to record if we had previously got a module with the same source as it built the tree, and then not try again if it had?

From #1418:

I think the proper long term solution is to have some sort of "module.lock" file that does this mapping for us so the hash doesn't matter at all.

From above:

as we do a bigger plan around how modules should be stored on disk

These two comments sound interesting, would be good to hear the plans for this if you're able to share.

mitchellh added a commit that referenced this pull request Sep 30, 2016
This changes the key for the storage to be the _raw_ source from the
module, not the fully expanded source. Example: it'll be a relative path
instead of an absolute path.

This allows the ".terraform/modules" directory to be portable when
moving to other machines. This was a behavior that existed in <= 0.7.2
and was broken with #8398. This amends that and adds a test to verify.
mitchellh added a commit that referenced this pull request Sep 30, 2016
This changes the key for the storage to be the _raw_ source from the
module, not the fully expanded source. Example: it'll be a relative path
instead of an absolute path.

This allows the ".terraform/modules" directory to be portable when
moving to other machines. This was a behavior that existed in <= 0.7.2
and was broken with #8398. This amends that and adds a test to verify.
@ghost
Copy link

ghost commented Apr 22, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

terraform module symlink error
4 participants