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

Add --modmandir option. #90

Merged

Conversation

pocallaghan
Copy link
Contributor

Specifying --modmandir allows control over where modman will place /
source code from. Effectively allowing multiple modman directories for a
single install. Refs #87.

@pocallaghan
Copy link
Contributor Author

@fbrnc I've tested a variety of different combinations, but let me know if you come across anything untoward.

# examples
modman init htdocs .modman-local
modman clone --modmandir .modman-local https://github.com/AOEpeople/Aoe_Profiler
modman undeploy --modmandir .modman-local Aoe_Profiler
modman deploy --modmandir .modman-local Aoe_Profiler
modman undeploy --modmandir .modman-local Aoe_Profiler
modman deploy-all --modmandir .modman-local
modman remove --modmandir .modman-local Aoe_Profiler

NB: Bash completion for the new option isn't currently supported. The command automodman also isn't supported since the command appears to crash for me.

@colinmollenhour
Copy link
Owner

I like the idea alright but the commands are very long. What about a syntactical improvement?

I propose if the module name includes a ':' the part before the colon is the alternate modman directory suffix (local = .modman-local) and the part after is the module name. E.g.:

modman deploy local:Aoe_Profiler
modman deploy-all local:  # or perhaps just $ modman deploy-all local

What do you think?

@pocallaghan
Copy link
Contributor Author

@colinmollenhour I see where you're coming from, it definitely felt a bit verbose as I was typing out the tests / examples. I'm certainly not opposed to the idea of using a different syntax, but I can foresee potential complications implementing the one you suggest. For example the second argument for all VCS commands will likely be a remote URL and thus could be considered ambiguous so if determined on a global scope as it is currently, then it is arguably a suffix of https as opposed to a url starting https. Not insurmountable though for sure.

@colinmollenhour
Copy link
Owner

Ok, so another character? Needs to not require shell escaping and not be a format used by git, hg or svn.. What about two colons?

$ modman deploy local::Aoe_Profiles

So the regex would be like (\w+)::(\w+). Fool-proof?

@colinmollenhour
Copy link
Owner

For the -all type commands I think the :: should be required to disambiguate between a module name and a namespace name. E.g.:

$ modman deploy-all local::

@fbrnc
Copy link

fbrnc commented May 4, 2015

Hi @colinmollenhour, hi @pocallaghan,
I strongly feel we should insert any more "magic" here. users will have to know how the directory name is generated and they would also be locked in into using a directory that starts with .modman. Although it's a little longer I think it's much better to design this as simple as possible and pass the raw directory name (that might also be a relative path to somewhere else!) as the option value.

Introducing a new syntax :: seems and teaching users how this works seems to be a high price for saving a couple of characters :) I'm all for removing this abstraction layer and keeping it simple...

Another use case that comes into my mind is you could have a directory of development modules on your local devbox that you keep at a central place and you share between mutliple projects.
The new --modmandir would be a great tool for this.

@colinmollenhour
Copy link
Owner

You could create a symlink named ".modman-shared" to your central location. Just sayin. ;)

One issue with the --modmandir case.. if a user wants to use only the alternate directory they need to have used "modman init" already to establish the location of the root. Also if the user later deletes the empty ".modman" directory because it is empty and they theink they don't need it, then the modman root will be unknown. Using a naming convention like I proposed means the directory/symlink itself would serve to establish the root even if no .modman directory exists (with a little work to support this scenario).

I'm open to either way, but this is definitely a feature that will be hard to make changes to in the future so just wanted to make sure it was discussed before it is merged.

@fbrnc
Copy link

fbrnc commented May 6, 2015

I never use modman init, but yes I think modman should assume you're using the default .modman dir for all operations unless you explicitly specify something different. And at this point you probably know what's going on.
If the user decides he doesn't need the default .modman dir he shouldn't expect commands like modman deploy-all to work without specifying any additional options.
I'm sure it's safe (or even the better design decision) to not implicitly generate the actual dir name based on a convention. After all user might even want to use composer's vendor directory directly with modman (assuming they're using an installer that doesn't store packages in vendor/<namespace>/<packagename> but on a single level: vendor/<packagename>)

Trying to stick to the unix concepts of having small and simple tools that do one thing and do that very well, I'm all for adding the --modmandir parameter, allowing it to point straight to any directory and make it the responsibility of the user to integrate it in his/her project structure and directory layout depending on his requirements.

Briefly looking at @pocallaghan, that's what's already happening in this pull request, right?

@colinmollenhour
Copy link
Owner

I agree about needing the flexibility, my point was just that the flexibility can still be achieved with the proposed shorter syntax by using symlinks. Another benefit would be that bash completion could be supported for the alternate module namespaces whereas with the currently implemented syntax there can be no autocompletion and it is much more lengthy and therefore more prone to typos. For example you might even forget which working copy of a package of modules you're working with whereas with a symlink pointing to it there can be no doubt/confusion. So for example:

Proposed:

ln -s vendor/AoeCommon .modman-AoeCommon  #only one time
modman deploy-all AoeCommon::

vs

modman deploy-all --modman-dir vendor/AoeCommon

@fbrnc
Copy link

fbrnc commented May 6, 2015

Hi @colinmollenhour,

good example! But to me the second option looks way more intuitive, doesn't require any knowledge on how AoeCommon:: translates into a directory and you don't have to deal with an extra symlink.
In my opinion that's totally worth the couple of extra characters you'd have to type :)

Specifying --modmandir allows control over where modman will place /
source code from. Effectively allowing multiple modman directories for a
single install. Refs colinmollenhour#87.
colinmollenhour added a commit that referenced this pull request Mar 11, 2016
@colinmollenhour colinmollenhour merged commit 5ee42c8 into colinmollenhour:master Mar 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants