← Back to team overview

kicad-developers team mailing list archive

Re: [Patch] Add gitlab support to github plugin

 

Parameters (or properties as they are defined internally) are passed
defined in the footprint library table.  They are simple key=value pairs
that are passed to the plugin defined by each footprint library table
entry.  The person that wrote this code thought that it would be more
effort to code a property than your original patch.  I also agree with
his assessment that looking for "git" in the server name is not a robust
solution.  The problem with your patch is every time someone wants to
support a git server with a different server naming convention, they
will want to add another server name check to the github plugin code
which IMO is not an acceptable solution.  I'm guessing you are using
Linux as your gitlab server.  One thing that has been suggested is to
use nginx on your server to act as a proxy between gitlab and the kicad
github plugin.  It should be possible to configure nginx to behave like
github so that your gitlab server looks like a github server to the
KiCad github plugin.  It could also be a lot faster since nginx can be
configured to cache the footprint libraries in memory.  The other option
is to maintain your patch until a more robust solution can be found that
will play nice with all git servers.

On 8/13/2015 10:42 AM, Ian Roth wrote:
> A parameter would work. I am not familiar with how to define new
> environmental variables in the code and what you would want it to be called.
> 
> On Aug 13, 2015 8:27 AM, "Wayne Stambaugh" <stambaughw@xxxxxxxxx
> <mailto:stambaughw@xxxxxxxxx>> wrote:
> 
>     The next obvious question is what happens when "git" is part of the
>     server name and the server doesn't follow the same zip file path
>     semantics that gitlab does?  This is my concern and the concern of the
>     person who wrote the github code.  This patch seems very specific to
>     gitlab rather than a generic patch for all server use cases.  My fear is
>     that everyone will want a patch specific to their git server api and the
>     code will spiral out of control.  I'm wondering if there is a better way
>     to handle this outside of the code as a plugin parameter rather than a
>     hard coded path.
> 
>     On 8/12/2015 12:57 PM, Ian Roth wrote:
>     > Gitlab uses different URLs for zip file downloads. Rather than
>     > <project>/zip/master the URLs are of the pattern
>     > <project>/repository/archive.zip. There are similarities in the two
>     > APIs, but they are not the same.
>     >
>     > The URL cannot be <project> as in the kicad code that handles non
>     github
>     > servers; that points to the project main page.
>     >
>     > On Wed, Aug 12, 2015 at 11:44 AM, Wayne Stambaugh
>     <stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>
>     > <mailto:stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>>> wrote:
>     >
>     >     Please help me understand the purpose of this patch.  Are you
>     saying
>     >     that the github plugin does not work when KIGITHUB has the
>     properly
>     >     specified URL and gitlab is configured properly?  What's different
>     >     between gitlab and github access that necessitates this change?  I
>     >     realize that it's a minor change but the comment in the source
>     says it
>     >     all and I would rather avoid polluting the source if there is an
>     >     external solution.
>     >
>     >     For future reference, please include you patch as an
>     attachment rather
>     >     than inline.  Bazaar is the VCS used by KiCad so inline
>     patches are more
>     >     work to apply.  The git repo is only a mirror for devs who
>     prefer git.
>     >     I noticed you created a pull request against the github mirror
>     which
>     >     will never happen because the github mirror is not the official
>     >     development repo.
>     >
>     >     On 8/8/2015 6:37 PM, Ian Roth wrote:
>     >     > This allows users to host the footprint libraries on a local
>     gitlab
>     >     > server in case there is no access to github.
>     >     >
>     >     > I know this patch has the potential to be controversial because
>     >     the code
>     >     > in the plugin is supposed to be as generic as possible and
>     that the
>     >     > server should change. I would like to counter that by noting
>     that
>     >     gitlab
>     >     > is probably the most popular option for self hosted git servers,
>     >     and the
>     >     > code change adds just one string comparison and a string
>     concatenation
>     >     > to make the integration work.
>     >     >
>     >     > This patch can be found online
>     >     > at: https://github.com/KiCad/kicad-source-mirror/pull/6/files
>     >     >
>     >     > Git diff:
>     >     > diff --git a/pcbnew/github/github_plugin.cpp
>     >     > b/pcbnew/github/github_plugin.cpp
>     >     > index 695365c..8ab03a2 100644
>     >     > --- a/pcbnew/github/github_plugin.cpp
>     >     > +++ b/pcbnew/github/github_plugin.cpp
>     >     > @@ -516,6 +516,14 @@ bool GITHUB_PLUGIN::repoURL_zipURL( const
>     >     wxString&
>     >     > aRepoURL, string* aZipURL )
>     >     >
>     >     >              zip_url += repo.GetPath();      // path comes
>     with a
>     >     > leading '/'
>     >     >
>     >     > +            // This is intented for use with gitlab, for users
>     >     who wish
>     >     > to self-host
>     >     > +            // the library repos on a local server. The
>     KIGITHUB path
>     >     > (and thus server name)
>     >     > +            // must contain "git" for this code to work.
>     >     > +            if ( repo.GetServer().Contains("git") )
>     >     > +            {
>     >     > +                zip_url += "/repository/archive.zip";
>     >     > +            }
>     >     > +
>     >     >              // Do not modify the path, we cannot anticipate
>     the needs
>     >     > of all
>     >     >              // servers which are serving up zip files
>     directly.  URL
>     >     > modifications
>     >     >              // are more generally done in the server,
>     rather than
>     >     > contaminating
>     >     >
>     >     >
>     >     > _______________________________________________
>     >     > Mailing list: https://launchpad.net/~kicad-developers
>     >     > Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>     >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx
>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>>
>     >     > Unsubscribe : https://launchpad.net/~kicad-developers
>     >     > More help   : https://help.launchpad.net/ListHelp
>     >     >
>     >
>     >     _______________________________________________
>     >     Mailing list: https://launchpad.net/~kicad-developers
>     >     Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>     >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx
>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>>
>     >     Unsubscribe : https://launchpad.net/~kicad-developers
>     >     More help   : https://help.launchpad.net/ListHelp
>     >
>     >
> 
>     _______________________________________________
>     Mailing list: https://launchpad.net/~kicad-developers
>     Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>     Unsubscribe : https://launchpad.net/~kicad-developers
>     More help   : https://help.launchpad.net/ListHelp
> 
> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 


Follow ups

References