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

Detect and configure Git updates from plugin headers only #16

Closed
wants to merge 1 commit into from

Conversation

pdclark
Copy link
Contributor

@pdclark pdclark commented Jun 29, 2012

This sets WPGitHubUpdater up as a self-configuring plugin that can either enabled in WordPress or included in another plugin.

Either way, the class enables Github updates for all plugins that use a github.com repository address as the Plugin URI in the header. For example:

/*
Plugin Name: WP Github Plugin Updater
Plugin URI: https://github.com/jkudish/WordPress-GitHub-Plugin-Updater
Requires: 2.8 (optional)
Tested: 3.4 (optional)
*/

The class doesn't just work for one plugin -- it searches and enabled all plugins. If searching Plugin URI would be inadvisable, an additional header item, like Git URI could be used instead.

To further reduce configuration, the plugin gets the latest version number and zip url from repository tags. The only required setup is a github repository URL.

@sc0ttkclark
Copy link
Contributor

I for one would like to see a separate header line for this, perhaps Github URI: https://github.com/jkudish/WordPress-GitHub-Plugin-Updater and maybe a line for Github Branch: master for example.

I like what you're after here though, I'm getting tired of changing two files :)

@ninnypants
Copy link
Contributor

I think having the URI and branch in their own lines makes sense though could also do something like below and just build the urls on the fly, but I can see how having the full github url could be useful.

Github User: jkudish
Github Repo: WordPress-GitHub-Plugin-Updater
Github Branch: master

@sc0ttkclark
Copy link
Contributor

Could be useful, as long as they are all within the plugin's headers themselves. We'll need to wait and see exactly what @jkudish wants to do here.

@pdclark
Copy link
Contributor Author

pdclark commented Jul 3, 2012

Configuration

Github URI: https://github.com/jkudish/WordPress-GitHub-Plugin-Updater

I'm partial to this format, for several reasons:

  • Reduced user error / Reduced complexity: You can't have typos when you didn't type: just copy-and-paste the URL.
  • Consistency with WordPress API and HTTP specs: Default WordPress plugins already use the Plugin URI form for updates, and URIs already define all necessary information. No reason to create a new syntax for existing functionality.
  • Easily extendable to multiple Git hosting services and private repos. (See below)

Multiple Git services

There's not much reason updates need to be restricted to just coming from Github-hosted repositories. I've already written code for private repos, and have researched writing connectors for generic Gitweb hosting. For example:

Github URI: https://github.com/jkudish/WordPress-GitHub-Plugin-Updater
Bitbucket URI: http://bitbucket.org/user/repo.git
Gitweb URI: http://git.your-site.com/some-repo.git

Private Repos

This format makes it trivial to support private repositories. Again, without additional header keys to memorize.

Private repos, one with Github oAuth, another with HTTP authentication:

Github URI: https://github.com/jkudish/WordPress-GitHub-Plugin-Updater?access_token=foobar
Gitweb URI: http://user:[email protected]/some-repo.git

Creating individual keys for every minor piece of information, like user, repo, and branch, password, or access token, then varied for different services, would have us up to 15 keys, just from the examples so far. I don't see any reason for that complexity, when the specification for a repository URL already contains all the necessary information in a standardized format, and PHP already has a very well-defined function for breaking a URL down into its individual parts. No need to force people to do a computer's job.

Tags work for any branch

AFAIK, there's not any reason to specify a branch because the Git method for identifying a new version, tags, is a global commit idenifier. That is, if I tag a commit as version 1.2, it doesn't matter if I did it on master, develop, or foobar. The commit will be the official release and prompt an update from the version number alone.

As an example, take a look at the Github API tags output for this repo. Branches don't come into play, because tags are global.

@jkudish
Copy link
Contributor

jkudish commented Jul 3, 2012

Howdy @pdclark, thanks for the contribution, this is awesome!

I'll have to give your pull requests a more in-depth look-over in the next few days before I merge them in. It's on my todo list so I'll get to it asap.

Thanks :)

@pdclark
Copy link
Contributor Author

pdclark commented Jul 3, 2012

Thanks for the positive feedback @jkudish ! I'm looking forward to hearing what you think. I realize the two merges aren't compatible with each other yet, but I wanted to get your approval before putting the time into merge them into a single final product. I'm happy to help do that– don't at all think I'm expecting you to stitch the two feature branches together!

@sc0ttkclark
Copy link
Contributor

+1

@jkudish
Copy link
Contributor

jkudish commented Jul 14, 2012

Howdy @pdclark et al,

This is the format I'd like to go with:

Github URI: https://github.com/jkudish/WordPress-GitHub-Plugin-Updater
Github branch: super-duper

If GitHub branch is left blank or not present, it should automatically default to master

I like the consistency that this format provides and as previously explained, it reduces the likeness for errors.

I don't however think that including private repos this way with the ?access_token in the URL is a good idea. I much prefer the approach that @pdclark took in the other pull request using oauth.

@pdclark can you make this happen, and work in coordination with your other pull request, and I'll gladly merge this in :)

@jkudish
Copy link
Contributor

jkudish commented Jul 14, 2012

Actually never mind about the branch thing, you are right, tags are best used in this case regardless of the branch and it's a much better approach.

@pdclark
Copy link
Contributor Author

pdclark commented Jul 14, 2012

Whew! Thanks for updating that so quickly! I was starting to stress out about branches again!

On the Github URI line, it's a fairly minor thing, but could we use Git URI: instead of Github URI ? There's not anything especially Github specific about the update service -- I've already successfully gotten updates to work from Generic Gitweb in my (rough!) branch f/gitweb

As for ?access_token vs. oAuth -- there's not any difference between the two. The setup assistance page from the pull request was formatting to match the existing config array, that just went straight to adding ?access_token= to the URL. http://github.com/user/repo?access_token=foobar is the official Github oAuth format.

@jkudish
Copy link
Contributor

jkudish commented Jul 14, 2012

On the Github URI line, it's a fairly minor thing, but could we use Git URI: instead of Github URI ?

Wouldn't the API used for each be different anyways? I'd rather stick to specific for now and use Github URI for now. We can always add a filter in the class itself and then you can use whatever keyword you want in your own implementation.

As for ?access_token vs. oAuth -- there's not any difference between the two.

I realize that the URL built is the same but I think from a security and maintainability point of view, it's better to generate the oAuth token on the fly instead of including it directly in the plugin's code that way. I guess, I just don't want to "advertise" doing it that way, but as you said, you could do it that way regardless :)

@pdclark
Copy link
Contributor Author

pdclark commented Jul 14, 2012

Github vs. Git

The API would be different, but transparent to the plugin. It's very similar to how wp_remote_get() works -- There's one generic wrapper class, but depending on what's requested and available, it transparently chooses between about 5 different HTTP transfer methods that all operate differently, but respond to the wrapper class in the same way. Consider this rough implementation from updater.php:170 in my f/gitweb branch:

public function get_repo_transport( $meta ) {
    if ( !empty( $meta['git uri'] ) ) {
        $parsed = parse_url( $meta['git uri'] );
    }else {
        $parsed = parse_url( $meta['PluginURI'] );
    }

    switch( $parsed['host'] ) {
        case 'github.com':
        case 'www.github.com':

            // The host is Github, so use the Github transport class

            if ( !class_exists('WordPress_Github_Updater') ) { include 'transports/github.php'; }
            list( /*nothing*/, $username, $repository ) = explode('/', $parsed['path'] );
            return new WordPress_Github_Updater( array_merge($meta, array( 'username' => $username, 'repository' => $repository, )) );
        break;
    }

    if ( '.git' == substr($parsed['path'], -4) ) {

        // This is a generic Git repository, so check for Gitweb and interface with the Gitweb transport class

        if ( !class_exists('WordPress_Gitweb_Updater') ) { include 'transports/gitweb.php'; }
        return new WordPress_Gitweb_Updater( array_merge( $meta, $parsed ) );
    }


    return false;
}

In this case, Git URI is looked at, and if the URL is for github.com exists, then a class for the proprietary Github API is used. Otherwise, if the URI ends in ".git", then a class that understands Gitweb (which is built into Git) is used. The same checks could detect bitbucket.org, beanstalkapp.com, or any other hosting vendor.

There are a few reasons why I prefer this approach over a filter. The third merits further clarification, so I've expanded it below.

  • Filter adds configuration complexity where it's not needed -- we already know that github.com uses the Github API, ".git" is generic, etc.
  • Github URI: http://github.com/xxx is redundant. The URI already says it's going to github.com
  • The difference between Git and Github is important. See below.

While github.com is certainly the most popular Git hosting service, there are certainly many others. I think the similarity in names makes it tempting to equate the two, but imagine if we were implementing any other update service for a version control system. Would we use Beanstalk URI instead of Subversion URI because Beanstalkapp.com is popular, then require a filter for users to un-brand their header or use another Subversion provider?

I know this may seem extremely nit-picky, but please consider it in the context of every time you've seen misuse of WordPress.com the company vs. WordPress.org the provider of self-hosted open-source software. This is very similar. There's nothing proprietary about the update service that's being provided by Github. Instead, we are interfacing with open standards provided by Git (tags, zip archives). For that reason, I think the distinction is important. At the very least we could support both lines to avoid user-confustion and forced-branding at the same time.

oAuth

Are you saying to add another different config line, like Access Token: foobar ? I think I'm somewhat wary of adding many keys, because each one then gets searched in every plugin each time plugins are scanned.

@jkudish
Copy link
Contributor

jkudish commented Jul 14, 2012

Github vs. Git

Fine, I'm okay with just having Git URI

oAuth

No, I don't think that the access token should be in the plugin header's at all, and only set via an option (which can be filtered if someone wants to set it permanently)

@pdclark
Copy link
Contributor Author

pdclark commented Jul 18, 2012

Setting oAuth authentication through an option hadn't occurred to me, but now that you mention it, it's obviously a better solution. It'll take me more time to setup the setup assistant to serve dual purposes -- generating a key and entering keys for different plugins. I'll put some thought into it while cleaning up that pull request.

@jkudish
Copy link
Contributor

jkudish commented Sep 13, 2012

Please reopen a new merge request if you have new work to share. Please try to keep things small and focused as that makes it easier to review and merge.

@jkudish jkudish closed this Sep 13, 2012
@pdclark
Copy link
Contributor Author

pdclark commented Sep 13, 2012

Good call. I'll squeeze it down to header config only when I get back to it.

@pdclark
Copy link
Contributor Author

pdclark commented Sep 24, 2012

I came back to this looking at how to rewrite the merge, thinking that I had included features besides configuration from the header, but then realized that it was only that one thing. With the other merge still pending, I'm not sure what to take as a next step. Was this merge rejected because it wasn't compatible with the other? Or because the changes were too significant?

I'd like to see single-line config merged into your copy, but by it's nature, it's a significant change. It'd be helpful for me in trying to meet your criteria if you could provide specific guidance on what you're looking for. What form would this change need to take for you to consider it for merge?

@jkudish
Copy link
Contributor

jkudish commented Sep 24, 2012

Was this merge rejected because it wasn't compatible with the other? Or because the changes were too significant?

If I remember correctly (sorry I could be wrong here), but the patch didn't apply cleanly against the repo. Other than that, it does seem like the changes attached to this pull request did change several things beyond just the way config is loaded

@pdclark
Copy link
Contributor Author

pdclark commented Sep 24, 2012

That may be the case; some of it was organizing in preparation for f/gitweb, which allows multiple transports depending on which Git service is being used.

In reworking it, should I go for a lean patch on your current master, or plan on f/oauth being merged in?

@jkudish
Copy link
Contributor

jkudish commented Sep 24, 2012

current please

On 2012-09-23, at 9:36 PM, Paul Clark [email protected] wrote:

That may be the case; some of it was organizing in preparation for
f/gitweb, which allows multiple transports depending on which Git service
is being used.

In reworking it, should I go for a lean patch on your current master, or
plan on f/oauth being merged in?


Reply to this email directly or view it on
GitHubhttps://github.com//pull/16#issuecomment-8807337.

@pdclark
Copy link
Contributor Author

pdclark commented Sep 24, 2012

If I'm not able to rewrite until next weekend or the weekend after, is there a chance f/oauth will be merged in then?

@jkudish
Copy link
Contributor

jkudish commented Sep 24, 2012

I need to review those changes. I doubt I will get to it this week. Let's
get this simpler set of changes in first :)

On 2012-09-23, at 9:42 PM, Paul Clark [email protected] wrote:

If I'm not able to rewrite until next weekend or the weekend after, is
there a chance f/oauth will be merged in then?


Reply to this email directly or view it on
GitHubhttps://github.com//pull/16#issuecomment-8807384.

@mastef mastef mentioned this pull request Sep 3, 2013
@mastef
Copy link

mastef commented Sep 3, 2013

@pdclark @jkudish do you mind writing up a quick tut or basic steps on how to get this plugin to run with a private bitbucket git repo?

@pdclark
Copy link
Contributor Author

pdclark commented Oct 30, 2013

@mastef Bitbucket support didn't end up getting merged into Joey's plugin, but I kept maintaining and using the code in the year since this pull request. It's now available publicly with a guide at https://github.com/brainstormmedia/git-plugin-updates

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.

5 participants