-
Notifications
You must be signed in to change notification settings - Fork 50
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
Different behavior between r10k and g10k when referencing Puppetforge module with git address #2
Comments
I acknowledge that this is a bug that does make g10k incompatible with r10k. My problem is that I am declaring a module a Puppetlabs Forge module if I encounter the owner/modulename syntax ( Lines 89 to 90 in cf471fd
That's why g10k is trying to fetch something from the forge API in this case. What I need to do to support the owner/modulename syntax for Git modules is to completely rewrite the Puppetfile config parsing, which is currently done with extremely ugly regexes. Start differentiating between Git and Puppetlabs Forge modules by looking at the given attributes. I'll try to do that in https://github.com/xorpaul/g10k/tree/fix_modulename The fact that the Puppetfile syntax is horrible to parse with anything except ruby where you can use the puppet library parser and the high maintenance/debug cost that this implies does make want to reject this bug and putting my effort into supporting a standard data format like JSON, YAML, TOML etc instead. For now please use mod 'php' for your Git module and we will see if I can rework the config parsing in the near future with justified effort. |
The "namespacing-feature" in r10k when handling Forge modules is not something that should be re-implemented when it comes to GIT, as the underlying reason for said namespacing does not exists there, afaik. Forge: Modules have to be addressed as "maintainer/module name", since the maintainer part of this is the only available differentiator when querying the forge - "team1/ntp" vs. "team2/ntp". So the namespacing there is fixed. Point being that carrying this over 1:1 from the Forge might be useful to some incarnations of git services (and even there only if the Puppetfile capabilities get vastly expanded upon), but definitely not all, so this can't be a "general g10k/GIT" feature, but a specific one for "g10k/gitlab | g10k/github". If this got implemented with no additional features around this, this does not seem to actually serve any purpose beyond keeping the status as a "drop in" replacement for r10k. In r10k this actually leads to bad/buggy behaviour right now - you can make r10k merge 2 different modules with the same name from different owners into the same target module directory, simply by combining in the same Puppetfile |
Well, indeed it doesn't seem to be a good idea to be consistent just for the sake of consistency when this results in such a bad merge behavior. However, I think a better error message than the current one would definitely be helpful, e.g. pointing out that the use of "maintainer/modulename" is not supported with Git addresses. But this still leaves the problem with syntax and regular expressions. ;) |
Yeah, I will definitely add better error handling for this. |
Puppet best practice is that all modules are named $org-$module, even for internal modules. You will see this if you run the metadata-json-lint on one of your modules. This bug makes g10k unusable and it is not a "drop-in" replacement. The documentation should be updated to reflect this. |
I just stumbled about a difference between r10k and g10k. Given the following definition in my
Puppetfile
:With g10k the following error appears:
With r10k instead, it will create simply drop the
mayflower/
part and create the folderphp
where it stores the module in.mayflower/php
is the official name of the Puppetforge module which I'd like to use for reference purpose, but as Puppetforge modules are sometimes out of date I like to reference the Github repository with the tag directly. Or, people use the forge module first and switch to the Github repo later but don't rename it in the Puppetfile and might run into this problem then.The text was updated successfully, but these errors were encountered: