Now reports differences on extra .git in the remote URL

[mvdan]

I now see a bunch of these for my own repos:

skipping "github.com/mvdan/gibot" because:
        remote URL doesn't match repo URL inferred from import path:
                  (actual) git@github.com:mvdan/gibot.git
                (expected) git@github.com:mvdan/gibot
skipping "github.com/mvdan/sh" because:
        remote URL doesn't match repo URL inferred from import path:
                  (actual) git@github.com:mvdan/sh.git
                (expected) git@github.com:mvdan/sh
skipping "github.com/mvdan/unparam" because:
        remote URL doesn't match repo URL inferred from import path:
                  (actual) git@github.com:mvdan/unparam.git
                (expected) git@github.com:mvdan/unparam
skipping "github.com/mvdan/git-picked" because:
        remote URL doesn't match repo URL inferred from import path:
                  (actual) git@github.com:mvdan/git-picked.git
                (expected) git@github.com:mvdan/git-picked
skipping "github.com/mvdan/goreduce" because:
        remote URL doesn't match repo URL inferred from import path:
                  (actual) git@github.com:mvdan/goreduce.git
                (expected) git@github.com:mvdan/goreduce
skipping "github.com/mvdan/xurls" because:
        remote URL doesn't match repo URL inferred from import path:
                  (actual) git@github.com:mvdan/xurls.git
                (expected) git@github.com:mvdan/xurls
skipping "github.com/mvdan/fdroidcl" because:
        remote URL doesn't match repo URL inferred from import path:
                  (actual) git@github.com:mvdan/fdroidcl.git
                (expected) git@github.com:mvdan/fdroidcl
skipping "github.com/mvdan/interfacer" because:
        remote URL doesn't match repo URL inferred from import path:
                  (actual) git@github.com:mvdan/interfacer.git
                (expected) git@github.com:mvdan/interfacer
skipping "github.com/mvdan/lint" because:
        remote URL doesn't match repo URL inferred from import path:
                  (actual) git@github.com:mvdan/lint.git
                (expected) git@github.com:mvdan/lint

Likely related to recent changes in reporting differing branches and remotes. These clones haven't changed in months, and I wasn't seeing these when reporting the previous issues a couple of weeks ago.

[dmitshur]

It looks like this is working as intended. It's detecting that there's a mismatch in the remote URL. Does gostatus show the same messages for those repos?

Are you okay with updating the remote URLs to not have the unnecessary ".git" suffix in remote URL?

[mvdan]

Does gostatus show the same messages for those repos?

Yes.

Are you okay with updating the remote URLs to not have the unnecessary ".git" suffix in remote URL?

It is unnecessary, but it's also irrelevant. GitHub adds it by default, which is why I have it in all of these.

In that sense, I don't think it should be on me to fix. A lot of users will come across this confusion. I seem to remember an issue where you said "/foo.git and /foo aren't the same URLs" but I don't remember the details.

In any case, they are the same if on GitHub.

[dmitshur]

A lot of users will come across this confusion.

Unfortunately, I think that's true. All right, I'll keep this open and think about it.

I seem to remember an issue where you said "/foo.git and /foo aren't the same URLs" but I don't remember the details.

Yes, that was https://github.com/shurcooL/gostatus/issues/37#issuecomment-225336823. It contains my rationale for why I don't want to support this.

In any case, they are the same if on GitHub.

Yes, if I add this, I will have to make it "github.com" specific. Which is a shame.

[mvdan]

Thanks - will re-read the discussion there and share my refined thoughts.

I'm still confused by how this is the first time I've seen these warnings on GPS. When I was reporting the other issues a couple of weeks ago, running GPS only yielded 3-4 warnings. Now it's more than a dozen, since all of the repos I have push access to are included because of this .git conflict.

[dmitshur]

I'm still confused by how this is the first time I've seen these warnings on GPS.

I'm puzzled by that too. I did change the logic of shouldPresentUpdate recently, where this check is performed. But all I did was re-arrange the order of checks.

I'm pretty sure the remote URL check was there since long ago, so I don't know why you weren't seeing these messages earlier. Perhaps there was a bug and it wasn't working as expected, but I can't easily spot anything wrong in older code.

For example, here it is, in a commit from 2 months ago:

https://github.com/shurcooL/Go-Package-Store/blob/21969781f65b431f1f0231dbe33f8842ce22c903/workspace/workspace.go#L587-L592

[mvdan]

I've shared some quick thoughts on your golang/go issue above.

I know you've talked about always ignoring .git before (https://github.com/shurcooL/gostatus/issues/37#issuecomment-225336823). But I still think it would be better to do it.

I see no benefit to supporting both ways, and so I'd rather support only one.

If the go tool supports both, and many people like me mix foo and foo.git, I see a clear benefit there.

gostatus is modeled after go get behavior, and so I don't want to add any additional flexibility on top of that.

If I understand things correctly, go get -u is already way more flexible as you described in the golang/go issue. It works on my repos where your tools error.

In the end, it's to keep it simpler, and allow less variability. Just like gofmt helps with keeping everyone's tabs vs spaces discussions at bay.

I can sympathize, but I would put this more in the realm of the robustness principle: https://en.wikipedia.org/wiki/Robustness_principle

It's fine for your tool to impose either version consistently, like go get does without .git, but both tools should accept both forms. Making humans appease the tools when both URLs just mean the same seems like doing things wrong to me.

(this was more of a reply to your thoughts in that gostatus comment, hence why they're here and not in the golang/go issue)

[dmitshur]

(this was more of a reply to your thoughts in that gostatus comment, hence why they're here and not in the golang/go issue)

This is the perfect place to do that. Let me reply to your arguments.

If the go tool supports both, and many people like me mix foo and foo.git, I see a clear benefit there.

To clarify, when this feature was originally added to Go in 1.4, it did not support both. See shurcooL/gostatus#24 for full details of when this feature was ported over from cmd/go into gostatus. It never had a special case for ignoring ".git" suffix. Instead, in CL 10693, it outright disabled any kind of import path checking for packages without an import path comment (which was later deemed as an invalid solution to the original problem).

If I understand things correctly, go get -u is already way more flexible as you described in the golang/go issue. It works on my repos where your tools error.

Yes, but only because of the bug. That was not the case when Go 1.4 first added the feature. See my comment in https://github.com/shurcooL/gostatus/issues/24#issuecomment-67449830.

Put simply, cmd/go did not support both when it was bug-free. I consider its current behavior buggy (but we'll see if Go team agrees), so an argument that the current version does something is not proof that it's the intended behavior.

https://en.wikipedia.org/wiki/Robustness_principle

I agree with the "be conservative in what you do" part of the robustness principle, but I strongly disagree with the "be liberal in what you accept from others".

Instead, I believe it's a much better strategy "to be strict about what you accept (i.e., follow the specification exactly), and report detailed error messages".

Reporting helpful and detailed error messages is absolutely critical. It means that faulty implementations will be detected and fixed during development, rather than proliferate, making codebases everywhere larger and more complex.

E.g., HTTP/2 implementations tend to be very strict and follow the spec, providing clear and actionable error messages to faulty peer implementations. This has been a huge success from what I've seen.

Making humans appease the tools when both URLs just mean the same seems like doing things wrong to me.

I have a new idea for how to deal with this. I agree that it's annoying, I just don't think ignoring ".git" for GitHub is the best way to solve it.

What if Go Package Store helped you correct instances of incorrect remote URLs with 1 click instead? Something like this (except better design):

To me, that's a much better fix. It lets the remote URL repo checking continue to be simple and free of special cases. But it also helps people in an automated way.

[mvdan]

Instead, I believe it's a much better strategy "to be strict about what you accept (i.e., follow the specification exactly), and report detailed error messages". [...] What if Go Package Store helped you correct instances of incorrect remote URLs with 1 click instead?

From my point of view (user) this solution is unfortunate, but I understand the same applies to ignoring .git from the point of view of the tools. In any case, having the tool fix it for me automatically (with the click of a button) would indeed be better than me having to do it one by one, and would probably keep bugs away in your tracker.

Just to understand, if your proposed changes in golang/go get accepted besides the .git ignoring part, then both GPS and go get would complain somehow about foo vs foo.git?

[dmitshur]

From my point of view (user) this solution is unfortunate,

I am finding it surprising that you still think that would be suboptimal for users, even after all the arguments presented here.

I really want to do everything possible from my side to find the best solution, and to minimize the chance of a misunderstanding causing me to settle for a suboptimal solution. To that end, let me restate things as I see them, and ask you @mvdan to confirm my understanding is correct.

As I understand, go get has always cloned GitHub repositories without the optional ".git" suffix (from Go 1.0 to today). So in a normal GOPATH workspace, starting with an empty one, filled up with Go packages over time via invocations of go get, all remotes will have no ".git" suffix. Even if that GOPATH workspace was created 8 years ago.

The only time that would be different is if people either manually cloned GitHub repositories with git clone and used ".git" suffix, or if they manually modified the remote URL of a github.com/... Go package with git remote set-url origin something.git.

So, if the ".git" suffix were to be disallowed, these people would only have to do these things:

1. Change only the repositories they've modified manually to include ".git" suffix (something tools like Go Package Store can help with), if any.
2. In the future, do not add ".git" suffix if cloning GitHub repositories manually instead of using go get.

That's it.

Is my my picture/understanding of the situation complete, or missing any factors? Is there any other reason why users would find it unfortunate?

Thanks for your help with this.

[mvdan]

I am finding it surprising that you still think that would be suboptimal for users, even after all the arguments presented here.

Perhaps my wording was a bit too extreme. I do agree with you, and I do think this solution is the correct one.

What I meant by "unfortunate" is that for a user that doesn't have the full picture, this solution might sound unnecessarily complex compared to "just ignoring .git". But as you explained, that solution is not as simple and consistent as it may initially seem.

If the text that GPS gives to remove the trailing .git explains this well, I don't think you'll get any users reporting this bug again :)

[dmitshur]

Thanks for explaining, I see what you mean.

I personally don't mind dealing with some reports and redirecting people to a solution. I care much more about the Go project remaining opinionated and simple, about making the better long term decisions, and not begin to cater to the least common denominator. It already has a history of being strict about things (tabs vs spaces, consistent case of acronyms, gofmt, build errors on unused imports, etc.), which not everyone likes at first, but it unites people and leads to a better and simpler future.

I really want to help maintain that as much as possible. But I realize the chances aren't great. :(

[mvdan]

Well, the chances are greater if you go with your solution rather than my initial one :)

[dmitshur]

I wanted to share one extremely minor point in favor of GitHub remote URLs without .git suffix being fine that I ran into.

Note the URL does not have a .git suffix, and the "ProTip" at the bottom says you can use the current page URL (without .git suffix) as the git remote URL.

[mvdan]

FWIW, I'm happy now obeying the rule that I should never use the trailing .git, at least on GitHub. So this can be closed as far as I'm concerned.

I'd still make one small change to GPS to notice this kind of error and replace it with a more appropriate one, to avoid this issue from being filed again.

[dmitshur]

I'd still make one small change to GPS to notice this kind of error and replace it with a more appropriate one

Can you elaborate on what you think would be a more appropriate error?

Currently, it's:

skipping "github.com/mvdan/gibot" because:
        remote URL doesn't match repo URL inferred from import path:
                  (actual) git@github.com:mvdan/gibot.git
                (expected) git@github.com:mvdan/gibot

Are you suggesting there should be a special case for github repos, such that if the difference in URL is a ".git" suffix, the error message should say something different? What should it say?

[mvdan]

Don't know if you would prefer replacing or extending the error, but I'd go with extending, like:

skipping "github.com/mvdan/gibot" because:
        remote URL doesn't match repo URL inferred from import path:
                  (actual) git@github.com:mvdan/gibot.git
                (expected) git@github.com:mvdan/gibot
        you should omit ".git" when cloning packages from GitHub.

[mvdan]

I'm happy with being consistent with my remote URLs now, so I'm going to close this.