Skip to content

Conversation

brunoga
Copy link
Contributor

@brunoga brunoga commented Dec 30, 2013

Note I am not sure how to do unit tests for this, but I am willing to do those if someone can point me to how to do it. Also I did not update the CHANGELOG as I am not sure where to put the change. Is it under v6.5.0?

 - Simply adds the releavant meta link in the project's show action.
 - Information about this meta tag can be found here:
   http://golang.org/cmd/go/#hdr-Remote_import_paths
@brunoga
Copy link
Contributor Author

brunoga commented Dec 30, 2013

@dosire
Copy link
Member

dosire commented Dec 30, 2013

This needs to use GitLab methods to link to the project (see the project model), otherwise stuff like relative url's will not work.

- This should work correctly in all cases now.
- Added a new project method that returns the web url for a project without
  the scheme prefix.
@brunoga
Copy link
Contributor Author

brunoga commented Dec 30, 2013

You are right. Changed it to use the web_url method. I had to add a similar method to get the project url without the scheme part.

@dosire
Copy link
Member

dosire commented Dec 30, 2013

You can't reply to issues

On Mon, Dec 30, 2013 at 9:19 PM, Bruno Albuquerque <notifications@github.com

wrote:

You are right. Changed it to use the web_url method. I had to add a
similar method to get the project url without the scheme part.


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

- Previus version was still incorrect as it ignored cases where GitLab would
  be installed in a custom location.
- This assumes that if this method can be calle, web_url will return a valid
  URL.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scheme? isn't it called protocol?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The formal name for this part of a URI is scheme. But I would be ok changing it to protocol if you prefer.

http://en.wikipedia.org/wiki/URI_scheme#Generic_syntax

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I learned something :-)

Please call it protocol since that is how the rest of the GitLab codebase refers to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@brunoga
Copy link
Contributor Author

brunoga commented Jan 20, 2014

Sorry for the delay. I was busy with real life. Things should move more quickly from now on.

@dosire
Copy link
Member

dosire commented Jan 20, 2014

Let me know if you need a pointer for the test.

@brunoga
Copy link
Contributor Author

brunoga commented Jan 20, 2014

Yes, please. I actually asked for pointers in a previous comment. :) In any case, here is what i said:

Ok, will do it. I searched but could not find where unit tests usually go in the gitlab codebase. Can you give me some pointers?

@dosire
Copy link
Member

dosire commented Jan 20, 2014

@brunoga I saw it in an email, I just couldn't find the comment here anymore :-)

Anyway, this should probably go in the project spec, just below other url functions in there https://github.com/gitlabhq/gitlabhq/blob/master/spec/models/project_spec.rb#L97

@brunoga
Copy link
Contributor Author

brunoga commented Jan 20, 2014

Got it. The test is done and i think it should work. How do I actually run gitlab tests to make sure it does?

@dosire
Copy link
Member

dosire commented Jan 20, 2014

@brunoga You can set up your own environment https://gitlab.com/gitlab-org/cookbook-gitlab/blob/master/doc/development.md or wait for the Travis tests to run.

@brunoga
Copy link
Contributor Author

brunoga commented Jan 20, 2014

ok. i will push the test so you can take a look at it. Meanwhile I will bring the development stuff up.

@dosire
Copy link
Member

dosire commented Jan 20, 2014

@brunoga OK, I'll probably wait for Travis to test it :)

@brunoga
Copy link
Contributor Author

brunoga commented Jan 20, 2014

Travis seem to like the CL:

https://travis-ci.org/gitlabhq/gitlabhq/builds/17274227

This probably only means Travis is drunk tough. ;)

@dosire
Copy link
Member

dosire commented Jan 21, 2014

@brunoga Thanks for adding the test!

@randx This would be great for Go users, can you look into this?

dzaporozhets added a commit that referenced this pull request Jan 21, 2014
 Added support for Go's repository retrieval.
@dzaporozhets dzaporozhets merged commit 5f9d90e into gitlabhq:master Jan 21, 2014
@brunoga
Copy link
Contributor Author

brunoga commented Jan 21, 2014

Thanks guys.

@brunoga brunoga deleted the go-repository-fetch branch January 21, 2014 10:36
@dosire
Copy link
Member

dosire commented Jan 21, 2014

@brunoga Thank you for contributing!

@randx Thanks for merging.

@dosire
Copy link
Member

dosire commented May 14, 2014

Two people suddenly report problems with this on GitLab.com https://twitter.com/schmichael/status/466254402693496832

https://gitlab.com/snippets/1118

@brunoga
Copy link
Contributor Author

brunoga commented May 14, 2014

Thanks for the heads up. The error is weird and I would need a pointer to a
repository where this is not working to be able to check what might be
happening.

On Wed May 14 2014 at 4:33:00 AM, Sytse Sijbrandij notifications@github.com
wrote:

Two people suddenly report problems with this on GitLab.com
https://twitter.com/schmichael/status/466254402693496832

https://gitlab.com/snippets/1118


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

@brunoga
Copy link
Contributor Author

brunoga commented May 30, 2014

So what happens is that HTML is not necessarily valid XML, so go get breaks
due to a "<" in that javascript code (which is interpreted as a tag being
opened). The best solution would probably be to check if the URL contains
?go-get=1 https://gitlab.com/jbowtie/ratago?go-get=1 and, if so, return a
simpler HTML("<meta ...>").

This should work but I will not be able to try to implement this in the
foreseeable future. Do you think you can pass this along to someone else
that could do it? it should not be that difficult (basically, use a
different template if go-get=1 is present).

On Fri May 30 2014 at 9:09:20 AM, Sytse Sijbrandij notifications@github.com
wrote:

Thanks for investigating Bruno!


Reply to this email directly or view it on GitHub
#5958 (comment).

@dosire
Copy link
Member

dosire commented May 30, 2014

@brunoga Thanks for investigating. This would have to be community contributed and I'm not sure the technical complexity is acceptable. Can the be solved at the Golang side?

@jbowtie
Copy link

jbowtie commented Jun 7, 2014

I've submitted a bug report on the Golang side (https://code.google.com/p/go/issues/detail?id=8163); they shouldn't be using an XML parser to process HTML5.

After looking through their code, a simple solution would be to move the generation of the meta tag to the top of the (or at least before the javascript is rendered).

@dosire
Copy link
Member

dosire commented Jun 18, 2014

@jbowtie I hope you are right, I submitted https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/128

@brunoga
Copy link
Contributor Author

brunoga commented Jun 18, 2014

This should work. Thanks for doing this.

On Wed Jun 18 2014 at 11:20:13 AM, Sytse Sijbrandij <
notifications@github.com> wrote:

@jbowtie https://github.com/jbowtie I hope you are right, I submitted
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/128


Reply to this email directly or view it on GitHub
#5958 (comment).

dzaporozhets added a commit that referenced this pull request Jun 23, 2014
@dosire
Copy link
Member

dosire commented Jul 2, 2014

Will go out in GitLab 7.1

@brunoga
Copy link
Contributor Author

brunoga commented Jul 2, 2014

Great.

On 1404289704625, Sytse Sijbrandij notifications@github.com wrote:

Will go out in GitLab 7.1


Reply to this email directly or view it on GitHub
#5958 (comment).

@jmunson
Copy link

jmunson commented Oct 6, 2014

I'm still having this issue (" XML syntax error on line 6: expected attribute name in element"), trying to use go get to install something from a private gitlab repo on gitlab.com.
The help page on gitlab says you're currently running GitLab Enterprise Edition 7.3.2-ee e4cb921 if that helps any.

@dosire
Copy link
Member

dosire commented Oct 8, 2014

@jmunson Someone will have to troubleshoot this.

@brunoga
Copy link
Contributor Author

brunoga commented Oct 8, 2014

James.

Can you, by any chance, post here (or mail me) the top of the HTML code for
the page in question? I just need the data up to (and including) the go
specific stuff.

Em Wed Oct 08 2014 at 11:59:25 AM, Sytse Sijbrandij <
notifications@github.com> escreveu:

@jmunson https://github.com/jmunson Someone will have to troubleshoot
this.


Reply to this email directly or view it on GitHub
#5958 (comment).

@jmunson
Copy link

jmunson commented Oct 8, 2014

Here is the results from a project used as an example earlier in the discussion, https://gitlab.com/jbowtie/ratago?go-get=1

I tried to paste the html, but I don't think github is fond of me pasting javascript into the comment box, so here's a gist: https://gist.github.com/jmunson/cfc501461467631fde24

@brunoga
Copy link
Contributor Author

brunoga commented Oct 8, 2014

So, this is exactly the same issue. There is some javascript code being
inserted BEFORE the go import directive and this is what is breaking
things. There are 2 things that might have happened:

1 - Somone did some change that added that code before the import
directive. The solution here is probably to move the import directive to
the top again and add a HUGE comment to the code mentioning nothing
should be added above it.
2 - This is the enterprise version of Gitlab. Maybe it did not pick up the
original fix for some reason?

Sytse?

Em Wed Oct 08 2014 at 2:12:18 PM, James Munson notifications@github.com
escreveu:

Here is the results from a project used as an example earlier in the
discussion, https://gitlab.com/jbowtie/ratago?go-get=1

I tried to paste the html, but I don't think github is fond of me pasting
javascript into the comment box, so here's a gist:
https://gist.github.com/jmunson/cfc501461467631fde24


Reply to this email directly or view it on GitHub
#5958 (comment).

@dosire
Copy link
Member

dosire commented Oct 8, 2014

Below is the current (088774b) state of EE (master)

Seems to be the top thing to me.

@jmunson What version are you on?

› git grep -C 15 go-import
app/views/layouts/_head.html.haml-%head
app/views/layouts/_head.html.haml- %meta{charset: "utf-8"}
app/views/layouts/_head.html.haml-
app/views/layouts/_head.html.haml- -# Go repository retrieval support
app/views/layouts/_head.html.haml- -# Need to be the fist thing in the head
app/views/layouts/_head.html.haml- -# Since Go is using an XML parser to process HTML5
app/views/layouts/_head.html.haml- -# #5958 (comment)
app/views/layouts/_head.html.haml- - if controller_name == 'projects' && action_name == 'show'
app/views/layouts/_head.html.haml: %meta{name: "go-import", content: "#{@project.web_url_without_protocol} git #{@project.web_url}.git"}
app/views/layouts/_head.html.haml- %meta{content: "GitLab Enterprise Edition", name: "description"}
app/views/layouts/_head.html.haml-

@jmunson
Copy link

jmunson commented Oct 8, 2014

Thats using whatever is live on gitlab.com, as of the time of me posting this that is currently GitLab Enterprise Edition 7.3.2-ee e4cb921

@dosire
Copy link
Member

dosire commented Oct 8, 2014

@jmunson Sorry, I missed that. I checked that commit but the git grep looks the same. The code inserted before contains beacon-2.newrelic.com so I think it is inserted by the newrelic gem.

@jacobvosmaer I don't think we use the browser time tracking from New Relic heavily and I don't like that it is inserting tracking stuff in our header. Shall we disable it?

https://ellislab.com/expressionengine/user-guide/monitoring/new-relic.html
To disable New Relic’s RUM JavaScript from being inserted, turn off the Enable New Relic RUM JavaScript? preference in the Output and Debugging settings.

@jacobvosmaer
Copy link
Contributor

I'm OK with not injecting NewRelic JS. I'll just have to figure out how to do that with environment variables.

@dosire
Copy link
Member

dosire commented Oct 9, 2014

@jacobvosmaer OK, thanks. Please comment back when done.

@jacobvosmaer
Copy link
Contributor

I added an env setting for New Relic (NEW_RELIC_BROWSER_MONITORING_AUTO_INSTRUMENT=false) and I now see no more JS before the meta content= declarations:

$ curl -s https://gitlab.com/jbowtie/ratago?go-get=1 | sed "/meta content='GitLab/q"
<!DOCTYPE html>
<html lang='en'>
<head>
<meta charset='utf-8'>
<meta content='gitlab.com/jbowtie/ratago git https://gitlab.com/jbowtie/ratago.git' name='go-import'>
<meta content='GitLab Enterprise Edition' name='description'>

@dosire
Copy link
Member

dosire commented Oct 9, 2014

@jacobvosmaer You mean you managed to disable RUM?

@jacobvosmaer
Copy link
Contributor

@dosire yes, I edited my comment above.

@dosire
Copy link
Member

dosire commented Oct 9, 2014

@jacobvosmaer Awesome, thanks!

@lfyzjck
Copy link

lfyzjck commented Oct 14, 2014

It seems that, golang can only recognized
<meta name='go-import' content='gitlab.com/jbowtie/ratago git https://gitlab.com/jbowtie/ratago.git'>
but not:
<meta content='gitlab.com/jbowtie/ratago git https://gitlab.com/jbowtie/ratago.git' name='go-import'>

I still got error: no go-import meta tags even if i have go-import meta tags

@jacobvosmaer
Copy link
Contributor

@lfyzjck I am getting the following with Go 1.3.3:

$ GOPATH=/tmp go get -v gitlab.com/jbowtie/ratago
Fetching https://gitlab.com/jbowtie/ratago?go-get=1
Parsing meta tags from https://gitlab.com/jbowtie/ratago?go-get=1 (status code 200)
get "gitlab.com/jbowtie/ratago": found meta tag main.metaImport{Prefix:"gitlab.com/jbowtie/ratago", VCS:"git", RepoRoot:"https://gitlab.com/jbowtie/ratago.git"} at https://gitlab.com/jbowtie/ratago?go-get=1
gitlab.com/jbowtie/ratago (download)
github.com/jbowtie/ratago (download)
<snip>

It looks like it's working on gitlab.com at the moment.

@lfyzjck
Copy link

lfyzjck commented Oct 14, 2014

@jacobvosmaer I have made a mistake, in my own project, i have make it's visibility to "Internal", when i call go get package, it just get a login page of gitlab. I have make it's visibility to "Public", and it just works.

thanks for you help!

@marciocarmona
Copy link

I'm facing the same problem @lfyzjck had. But in my case I can't change my repo visibility to public.
I guess maybe GitLab should return the HTML with the meta tag when it receives the go-get=1 parameter instead of returning a 302 to the login page, or just starting accepting and redirecting the URLs adding the ".git" extension automatically

@marciocarmona
Copy link

Apparently, #7693 will fix that! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

9 participants