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

A suggestion for autocreation of modman file #68

Open
waynetheisinger opened this issue Jul 13, 2014 · 26 comments
Open

A suggestion for autocreation of modman file #68

waynetheisinger opened this issue Jul 13, 2014 · 26 comments

Comments

@waynetheisinger
Copy link
Contributor

Hi Colin,

This is a suggestion, not an issue. How about an autocreate for the modman file? This would be very useful for when people retroactively create a modman file for their module rather than writing one when developing, maybe because it is a third-party module rather than their own.

How it would work

  1. -bash$ modman automodman
  2. the script would traverse the directories below the one it was called from
  3. at each directory level it would print the path and ask include 'folder/path', [y] [n]
  4. if the answer was no but the folder contained files it would then loop through the files asking include folder/path/filename [y] [n]
  5. If the answer was yes to either of the above it would create a simple modman line where both left and right path matched (I think that if users are going have a different folder structure in their modman folder to that in Magento this would still save them half the work of creating a modman file.)
  6. it would proceed to the next directory, either below its current level, or at a sibling level, or by returning to the parent and traversing down to the next in sequence. At each level the process 1 to 4 would be repeated
  7. control C would break operation

What do you think?

@waynetheisinger
Copy link
Contributor Author

If you think it would be useful perhaps we could collaborate?

@kubaceg
Copy link
Contributor

kubaceg commented Jul 21, 2014

  • it could detect any conflicts with existing in Magento dirs/symlinks
  • it should ask include 'folder/path', [Y] [n] and Y is default option (just hit enter)

@waynetheisinger
Copy link
Contributor Author

interesting, the [Y] [n] point is obviously straightforward.

Your first point assumes that you would running modman automodman from a Mage site. Presently a Modman checkout requires that a modman file already exists, do you foresee that you would use svn checkout instead?

Personally I was assuming that users would be running modman automodman in a folder tree outside of a Mage site before their first commit - though I think that this alternative workflow would avoid conflicts and therefore be very nice.

@waynetheisinger
Copy link
Contributor Author

thinking about it, it could be run in two modes,

  1. as a command inside a folder lacking a modman file that was contained within the basedir of the modman deploy root, (how that folder got placed there would be up to the developer probably as the result of a move or copy command).
  2. or as an --automodman option of modman checkout, if the option was present modman would not require a modman file and would instead use the process to create one.
    For safety I don't think that it should create the symlinks unless the --force option was used, instead it should break after the modman file is created to allow the developer to check the modman file.

@waynetheisinger
Copy link
Contributor Author

I'm thinking of creating a UML activity diagram to show my proposed flow, which I'll post that back here incase anyone wants to comment.

@waynetheisinger
Copy link
Contributor Author

Here is the Activity Diagram for modman automodman
modmanactivity

@riconeitzel
Copy link

w00t :D

@waynetheisinger
Copy link
Contributor Author

Here is the Activity Diagram for modman checkout modulename --automodman [--force]
modmanactivityasoption

@waynetheisinger
Copy link
Contributor Author

I'm going to start work on coding this and will and will do a pull request when I'm finished

@waynetheisinger
Copy link
Contributor Author

erm... --force always seems to be true for a checkout as its being set on line 870

@waynetheisinger
Copy link
Contributor Author

Right use my own --autoforce option
modmanactivityasoption

@waynetheisinger
Copy link
Contributor Author

Logic Structure as comments prior to coding it

# returns 0 on success 1 on error
    # given path by option?
        # no
            #where are we?
                #project root
                    #error
                #base directory
                    #error
                #a module folder
                    #set module
        # yes
            #set module
            #does the modman file exist?
                #yes
                    #echo file exists and exit without error
                #no
                    #touch modman
            #loop through files  (banned include modman and hidden files)
                #pre-existing path as part of ananother module
                    #yes
                        #error
                    #no
                        #include path yes no
                            #yes
                                #write path to modman
                                    #was it a directory or file
                                        #file - continue
                                        #directory - stop modman recursing
                            #no
                                #continue
                        #no more paths
                            #return success

@waynetheisinger
Copy link
Contributor Author

On line 349 in function apply_modman_file

# Assume target == real if only one path is given
if [ -z "$real" ]; then
    real="$target"
fi

This seems to suggest that only one half of the path is needed - though I can't find this documented on the wiki

@waynetheisinger
Copy link
Contributor Author

Think I'll create both sides anyway as I think this is what most people will expect to see

@waynetheisinger
Copy link
Contributor Author

I'm changing the [Y][n] default to [N][y] - I've found that when traversing down a tree, say:
app/code/community/module
You say n three times before you say Y on the last folder, therefore N is the more common option

@waynetheisinger
Copy link
Contributor Author

testing if the pathname exists in another modman file doesn't test if a parent directory exists in another modman - I'll think about that over the weekend

@waynetheisinger
Copy link
Contributor Author

I've worked it out - automodman now tests if parent exists and tells you

A path was found that exists in below modman description file
/var/www/magento/.modman/sweetooth/modman:app/code/community/TBT  app/code/community/TBT
It is therefore presently illegal for it to be included in this module.

press enter to continue...

@waynetheisinger
Copy link
Contributor Author

As you can see above anyone wanting to try out this code its now on pull request #69

@joh-klein
Copy link

Nice. Maybe you add a "simple" mode, where it just adds every file it finds. I am doing find . -type f > modman (inside the module) as a first step right now …

@brb3
Copy link

brb3 commented Feb 20, 2015

I have another take on this as a ruby script here.

@waynetheisinger
Copy link
Contributor Author

@bobbyburden there are indeed many ways to remove the proverbial four legged feline mammal from its epidermis 😉

@joh-klein could be useful - I also considered the filename* glob pattern as way of achieving the adding of every file, and I'll probably get round to adding it to my fork the next time it's useful to me...

ie

include '/this/is/your/path/': [N][y][*]

which would create

/this/is/your/path/*  /this/is/your/path/*

@colinmollenhour
Copy link
Owner

@waynetheisinger First of all, I am very impressed with your thoroughness. I think this is a good feature and it looks like you've implemented it really well and it doesn't break anything or fundamentally change anything. So, the only reason I haven't merged it is I haven't had time to test it out and look at it more closely, then I forgot about it.. So, sorry for that, I didn't mean to ignore your work.

I will add one thing in general is I like to stay away from building in too many features and options. For example, if a user wants to checkout/clone a repo that doesn't have a modman file, then they can already easily do this by:

$ cd .modman
$ git clone ... {module}
$ cd {module}
$ modman automodman

So I don't see the need in this case to complicate the checkout/clone command.

@waynetheisinger
Copy link
Contributor Author

@colinmollenhour you're probably right about not needing to change the checkout/clone command

Also my fork has gone a little stale so if you are interested in doing a merge then I'll get mine up to date with your trunk and remove the checkout/clone code

There is also a small bug that I want to fix - it gives a false positive in the error checking for pre-existing paths in other modules, if comments in the other module, match the path that automodman is testing against - I think it should only throw the error for actual modman paths not those that are commented out....

I'll fix that too and then do another pull request

@colinmollenhour
Copy link
Owner

Yes, I am willing to merge this feature after said changes. Thanks!

@waynetheisinger
Copy link
Contributor Author

@colinmollenhour I've made those changes and sent through another pull request... Thanks

@tmotyl
Copy link
Contributor

tmotyl commented Sep 17, 2015

just for the sake of completeness, here is a link to the tool already capable of generating modman files:
https://github.com/mhauri/generate-modman

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

No branches or pull requests

7 participants