Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

[PATCH] Passing a --name to script/plugin install and other improvements.

#3197

Hi,
Just enhanced some shortcomings of command script/plugin install --.
Most of the examples are of my own plug-in on which I have been testing the written code.

#1 - Name of the plugin is usually the last fragment of the URL (or second last if last is trunk) and there is no option to change that. Ex: -

script/plugin install http://kopal.googlecode.com/hg/

will install the plugin in vendor/plugins/hg/ directory.

Hacking like - http://kopal.googlecode.com/hg/?hack=/kopal/, won't work either. Because, script will then try to fetch http://kopal.googlecode.com/hg/?hack=/kopal/lib/, hence making an infinite loop.

Added a "--name" option for the command, so

script/plugin install http://kopal.googlecode.com/hg/ --name kopal

will install the plugin in vendor/plugins/kopal directory.

#2 - For a plugin URL with no post-fixed forward slash "/", if the server doesn't redirect to a URL with trailing "/" (ex: Google Code with Mercurial repository), a "Plugin not found" message is shown.

For example,

script/plugin install  http://kopal.googlecode.com/hg ;#without post-fixed forward slash "/"

will result in the first link hg/lib/, which when passes through File.join() with base_url, resulting http://kopal.googlecode.com/hg/hg/lib/, which raises a 404 error.
This is a Mercurial web-end specific error as usually all Subversion web-ends will redirect to a URL with trailing '/'.
For example - visiting http://websvn.kde.org/trunk will redirect to http://websvn.kde.org/trunk/ or http://freenet.googlecode.com/svn will redirect to http://freenet.googlecode.com/svn/.

Fixed. So now all URLs passed to RecursiveHTTPFetcher are post-fixed with '/' unless one exists. This doesn't create a new limit to RecursiveHTTPFetcher#new to only accept directory paths and not file paths, since it was already limited, as (before) RecursiveHTTPFetcher#fetch treated all initial URLs as directory.

#3 - If the Plugin URL contains GET parameters, A "Plugin not found error" is thrown or the script goes in infinite loop. Ex:

script/plugin install http://kopal.googlecode.com/hg/?r=tip

will result in RecursiveHTTPFetcher#fetch_dir() asking for http://kopal.googlecode.com/hg/?r=tip/lib/?r=tip resulting in a 404 error.

For subversion repository, this instead results in a infinite loop. since most subversion web-ends will parse ?r=42/lib/ as revision#42 and will not send a 404. (Ex: http://freenet.googlecode.com/svn/?r=42/anything will fetch revision#42).

Note - Following are some bugs/features I discovered. Didn't code, since I didn't find them much significant but worth mentioning here.

#5 - RecursiveHTTPFetcher only fetches sub-paths relative to current path, raises error for relative to domain paths and doesn't fetches absolute paths at all (talking only about files in the desired hierarchy).

Ex:
If http://example.org/svn/myplugin/ contains three links http://example.org/svn/myplugin/absolute/, /svn/myplugin/rel-domain/ and rel-current/,

script/plugin install http://example.org/svn/myplugin/

will fetch rel-current/, raise a 404 error while fetching rel-domain/(interprets it as http://example.org/svn/myplugin/svn/myplugin/rel-domain/) and won't fetch absolute/ at all.

However, this is not of much significance, since almost all revision-control-system web-ends publish links in relative-to-current-path format.

#6 - While downloading a plugin from web, some directory path, which may not be in interest of end-user should be ignored by default. Ex: /test/*, /.gitignore, /.hgtags, /.hgignore etc. (Usually most of them are in the root directory of a plugin).

NOTE: While I tested my modifications intensively, I didn't write test for them, since can't find any existing test which I can edit.

Willing to implement/fix last two features/bugs if the community finds them significant and gives a go.

PS: This is my first ever contribution to an open-source software.

Reported by Vikrant Chaudhary · September 14th, 2009 @ 08:54 AM

State: stale
Milestone: none
Assigned to: nobody
Importance: none

Activity

  1. Vikrant Chaudhary
    Vikrant Chaudhary
    • Tag changed from command, plugin to command, patch, plugin

    Bump.

    September 17th, 2009 @ 01:13 PM

  2. CancelProfileIsBroken
    CancelProfileIsBroken
    • Tag changed from command, patch, plugin to bugmash, command, patch, plugin

    September 25th, 2009 @ 12:16 PM

  3. Elomar França
    Elomar França

    +1, verified.

    The given patch applies on 2-3-stable. I've attached a patch that applies on master.

    It works, but I think it could be a little cleaner and more "railsish".

    September 27th, 2009 @ 08:45 PM

  4. Gaius Centus Novus
    Gaius Centus Novus

    +1 I made your version a little more Ruby-ish, Elomar.

    September 27th, 2009 @ 09:44 PM

  5. Blue Box Stephen
    Blue Box Stephen

    +1 verified

    Gaius Centus Novus' patch applies and tests cleanly to master.

    September 27th, 2009 @ 10:25 PM

  6. Vikrant Chaudhary
    Vikrant Chaudhary

    Thank you all for responses. I'm new to Git. I was wondering how can I learn the differences made by Elomar França and Gaius Centus Novus to my code that makes it more "rails-ish" and "ruby-ish".

    September 30th, 2009 @ 07:55 AM

  7. Rizwan Reza
    Rizwan Reza
    • Tag changed from bugmash, command, patch, plugin to command, patch, plugin

    February 12th, 2010 @ 12:46 PM

  8. Santiago Pastorino
    Santiago Pastorino
    • State changed from new to open
    • Importance changed from to

    This issue has been automatically marked as stale because it has not been commented on for at least three months.

    The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

    February 2nd, 2011 @ 04:55 PM

  9. Santiago Pastorino
    Santiago Pastorino
    • State changed from open to stale

    February 2nd, 2011 @ 04:55 PM