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

Bug: duplicate resource due to aliasing/namevar having filepath stripped off #40

Closed
JoshiiSinfield opened this issue Apr 10, 2015 · 5 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@JoshiiSinfield
Copy link

Hi,

I'm using archive to copy the same file into multiple different locations, however I've recieving the following error:

==> : Error: Cannot alias Archive[/tmp/result2/result] to ["result"] at /tmp/vagrant-puppet/manifests-846018e2aa141a5eb79a64b4015fc5f3/manifest.pp:25; resource ["Archive", "result"] already declared at /tmp/vagrant-puppet/manifests-846018e2aa141a5eb79a64b4015fc5f3/manifest.pp:18

Here's the code to reproduce it:

$source_file = '/tmp/source'

file { $source_file:
  ensure  => file,
  content => 'this is a test'
}

file { ['/tmp/result1', '/tmp/result2']: ensure => directory }

archive { "/tmp/result1/result":
  ensure  => present,
  name    => "/tmp/result1/result",
  source  => "file://${source_file}",
  extract => false,
  require => [File[$source_file], File[['/tmp/result1', '/tmp/result2']]]
}

archive { "/tmp/result2/result":
  ensure  => present,
  name    => "/tmp/result2/result",
  source  => "file://${source_file}",
  extract => false,
  require => [File[$source_file], File[['/tmp/result1', '/tmp/result2']]]
}

I've got a working solution which I hope will be acceptable. I'll fork and submit a pull request.

Thanks,
Josh

@nanliu nanliu added the bug Something isn't working label Apr 10, 2015
@nanliu
Copy link
Contributor

nanliu commented Apr 10, 2015

Interesting bug based on your use case. I can't always assume the same filename is the same file, but I'm trying to understand if it makes sense to download the same file twice. Would it be more effective to download one archive and use the file resource to place it in multiple location (unless you need extraction). I'll give it some thought.

Your PR is currently failing spec test, so can not be merged.

@JoshiiSinfield
Copy link
Author

it doesn't make sense to download the same file twice, you're quite right. I am currently downloading one file to /tmp and using the file resource to place it in multiple locations yes.

originally I was using archive to download it multiple times - putting a jar file into specific webapps//WEB-INF/lib/ on tomcat - which depends on the dirs being created already - by tomcat exploding the war. And then i switched to using the one archive download, and file resources to place it in the correct locations, however I was getting issues with directories not pre-existing. Which I then thought "oh, archive must do a mkdir_p or something similar as it was working previously with the multiple archives downloading straight to the WEB-INF/lib folder" however it seems I'm wrong, and have issues with my resource orderings, timings etc.

This still seems to be a bug, even though how I came about it is not my actual use case anymore!

Thinking about your comments, I think it'd be wise to create a temp filename off a UUID? that way you will always know the temp file created there and then has a unique name?

@nanliu
Copy link
Contributor

nanliu commented Apr 13, 2015

Yep, agree definitely a bug.

@nanliu
Copy link
Contributor

nanliu commented Apr 13, 2015

So giving it more thought, the fix for this issues is to change the full filepath :path to namevar, but that's going to be a major breaking change, because path was intended to be the file directory not the full file path. To preserve the existing module behavior

  1. namevar does not get transformed to filename.
  2. munging of namevar extracts path and filename.
  3. use some form of uuid for tempfile (maybe cache by checksum & checksum_type)

I think this will preserve backwards compatibility, but I really wish I didn't have to deprecate and move to a different resource parameter that's somewhat inconsistent with Puppet file.

@nanliu
Copy link
Contributor

nanliu commented Apr 23, 2015

After reviewing :path is actually full path for file, so this was not as big a change as originally expected.

This is fixed in #47. I believe it's backwards compatible (the archive namevar change, not the archive class changes). Please verify and review the 0.3.x branch.

@nanliu nanliu closed this as completed Apr 23, 2015
@nanliu nanliu added this to the 0.3.0 milestone Apr 23, 2015
@nanliu nanliu self-assigned this Apr 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants