modman icon indicating copy to clipboard operation
modman copied to clipboard

A suggestion for autocreation of modman file

Open waynetheisinger opened this issue 10 years ago • 26 comments

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 avatar Jul 13 '14 07:07 waynetheisinger

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

waynetheisinger avatar Jul 14 '14 12:07 waynetheisinger

  • 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)

kubaceg avatar Jul 21 '14 06:07 kubaceg

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 avatar Jul 21 '14 09:07 waynetheisinger

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 avatar Jul 22 '14 09:07 waynetheisinger

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 avatar Jul 22 '14 10:07 waynetheisinger

Here is the Activity Diagram for modman automodman modmanactivity

waynetheisinger avatar Jul 24 '14 08:07 waynetheisinger

w00t :D

riconeitzel avatar Jul 24 '14 09:07 riconeitzel

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

waynetheisinger avatar Jul 24 '14 10:07 waynetheisinger

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

waynetheisinger avatar Jul 24 '14 10:07 waynetheisinger

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

waynetheisinger avatar Jul 24 '14 16:07 waynetheisinger

Right use my own --autoforce option modmanactivityasoption

waynetheisinger avatar Jul 24 '14 16:07 waynetheisinger

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 avatar Jul 25 '14 07:07 waynetheisinger

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 avatar Jul 25 '14 09:07 waynetheisinger

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

waynetheisinger avatar Jul 25 '14 09:07 waynetheisinger

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 avatar Jul 25 '14 15:07 waynetheisinger

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 avatar Jul 25 '14 17:07 waynetheisinger

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 avatar Jul 26 '14 08:07 waynetheisinger

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

waynetheisinger avatar Jul 28 '14 12:07 waynetheisinger

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 …

joh-klein avatar Feb 19 '15 12:02 joh-klein

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

brb3 avatar Feb 20 '15 13:02 brb3

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

@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/*

waynetheisinger avatar Feb 20 '15 14:02 waynetheisinger

@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.

colinmollenhour avatar Feb 23 '15 17:02 colinmollenhour

@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

waynetheisinger avatar Feb 23 '15 18:02 waynetheisinger

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

colinmollenhour avatar Feb 24 '15 03:02 colinmollenhour

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

waynetheisinger avatar Mar 02 '15 11:03 waynetheisinger

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

tmotyl avatar Sep 17 '15 14:09 tmotyl