Add ability to use custom branch from vcs config for git

[pboutes]

This PR allows us to use a custom branch for the git driver by setting a ref property in the vcs-config object.

Example:

    "repos" : {
        "Hound" : {
            "url" : "https://www.github.com/etsy/Hound.git",
            "vcs-config": {
                "ref": "custom"
            }
        }
    }

If no ref or vcs-config properties are provided, the branch which will be targeted will be master.

[dmsimard]

This works for us and was the feature I had been anxiously waiting for a long time, thank you VERY MUCH! I'll share an example implementation soon, but in the meantime I'll go ahead and cross-reference the different pull requests and issues that are about this topic.

[mikepurvis]

#213 has worked for us. With dozens of open issues and PRs that have languished for months, I think this repo is basically abandonware— it would be great if someone could fork it and take over active maintenance.

[kellegous]

Recently some of other folks have been starting to contribute to hound, which is great and I hope it will get us back to a place where we can effectively maintain it. Sadly, I am the only remaining maintainer and I no longer work at Etsy which makes it difficult to keep up here.

On this particular issue, I think the change for git is ready to merge but the other VCS drivers should also use this part of the config. Honestly, I wish hound had only ever supported git, but now that the other VCS drivers are there, they should be supported. Is anyone interested in adding branch support for the other VCS drivers?

[avar]

@kellegous Not that custom branch support isn't also nice, but I wonder if most people who'd want this don't just want hound to stop obeying what upstream HEAD points to, which it did before #221

Which is one of many issues with Hound's Git usage as noted in #249

Given that hound has no support for indexing multiple branches in the same repo, I find it implausible that the real blocking issue isn't just that #221 should be mostly reverted, as opposed to someone wanting to index a topic branch and not what you get by default with git clone.

[dmsimard]

@kellegous maybe it would be worth considering asking Etsy if they are interested in splitting hound into it's own organization if they are not going to maintain it ?

And then, once the new organization is there, you're free to onboard new contributors/maintainers who can go through those issues and pull requests.

FWIW, the OpenStack open source community uses Hound to index ~2000 repositories between codesearch.openstack.org and codesearch.rdoproject.org so Hound is quite important to us.

[sahildua2305]

@dmsimard thanks for your comment. Hound is used a lot at my current company and it's quite important for us as well. That's why I reached out to @stanistan from Etsy to ask about the possibility of transferring the ownership/committer-ship for the project.

@stanistan can you share your thoughts here?

[legoktm]

The hound instance I set up for the MediaWiki project has become pretty popular, and we have an interest in seeing this project flourish :) If etsy is no longer interested in maintaining it, should we fork it into a separate "hound" organization? I've already set up my own fork for this patch, but I'd much rather join with a community to maintain this instead of going on my own.

[dgryski]

@legoktm I'd file a separate bug about transferring ownership to a new organization. If @kellegous doesn't have time maybe we could contact some other Etsy engineering staff here or on twitter.

[Wilfred]

We'd also love to see this feature landing. It'd be great as-is, and it doesn't prevent adding support for other VCS systems in future.

Looking at the code, I don't think this (yet) enables multiple checkouts of the same repo at different branches, as the vcs-abcdef12345 folders are only the hash of the VCS URL, excluding the branch. That would also be nice to explore in the future.

[grahamc]

One note is this PR doesn't properly link to the source, because the baseUrl is still master. You can update your config per-repo, or apply this patch:

From 6b4c532f46e88f93d3619f7e3c5983f573510714 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Tue, 10 Jul 2018 12:55:59 -0400
Subject: [PATCH] Replace master in the default base URL with a rev

---
 config/config.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/config.go b/config/config.go
index ebda60d..b4a29fb 100644
--- a/config/config.go
+++ b/config/config.go
@@ -13,7 +13,7 @@ const (
 	defaultPushEnabled           = false
 	defaultPollEnabled           = true
 	defaultVcs                   = "git"
-	defaultBaseUrl               = "{url}/blob/master/{path}{anchor}"
+	defaultBaseUrl               = "{url}/blob/{rev}/{path}{anchor}"
 	defaultAnchor                = "#L{line}"
 )
 
-- 
2.16.4

[valentin-krasontovitsch]

Just to push this again - I would suggest to merge this as a git-only feature.

It doesn't seem to be a priority for other VCS, as nobody came forward with an implementation, and I think it's a bit unfair to hold back this feature for git users.

What is needed for this to be merged now, perhaps with the patch that @grahamc suggests?

[pboutes]

Code has been updated with the suggested @grahamc patch

[genuinelucifer]

I have successfully adapted and tested a patch for specifying multiple branches in all the repos. I have also added an option to specify if user wants to search all branches or only master branch. Anyone interested can find the patch here:
https://gist.github.com/genuinelucifer/77346b81f79d2fdf3853fec16ef32fc0

Putting this here in case someone lands here from google (like I did 1 month ago).

[yelizariev]

@kellegous do you have admin rights to grant write access to some community members?

[mnacharov]

@kellegous do you have admin rights to grant write access to some community members?

@jklein @kellegous Please, find maintainer for this repo!

I't would be great to see this feature in mainstream

[mnacharov]

I think usage example in config-example.json won't be redundant

[ianfixes]

What work remains to be done to get this feature merged?

[salemhilal]

Hey @ianfixes (and everyone else), this PR might have been superseded by this one:
#345

Do you mind confirming?

I think this PR predated my involvement with the project, so I'm sorry if this PR has been left open longer than it should have been.

[ianfixes]

I see the issue.

The hound project itself has switched its default branch from master to main, and for whatever reason I was unable to pick up the change in my local fork by git merge origin/master (despite the fact that master seems to have been the merge target for #345).

I will open a separate issue for the existence of the master branch.

[salemhilal]

@ianfixes Gotcha. Just to be clear, origin/master doesn't exist anymore — it was renamed to main. Existing PRs (like this one) were updated to point to main. I think I'll need more details on what issue you're seeing regarding the existence of master, but it's unlikely that we're going to recreate master as a branch.

[ianfixes]

I was going to open an issue to say that master should be deleted if it's not going to be used... but when I looked, there is no such branch! https://github.com/hound-search/hound/tree/master

I think that what had happened was that my local git instance still had the concept of a master branch tied to this repo, so it just got stuck wherever it was the last time I fetched from upstream. I'm sure there is a git command that would have straightened out some of that for me, and at best left me with a "no such branch called master" error

[ianfixes]

I can confirm that #345 has produced desired functionality, thanks. The only thing it doesn't seem to do is produce /blob/branchanme in the URL -- it gives the SHA1 instead. Still, my needs are met here.

[salemhilal]

@ianfixes oh awesome! I was worried that you needed master specifically for some reason. 😅

Can you tell me more about the config overrides you need (is that work for this repo or yours)?

Either way, it sounds like I might be able to close this PR, but I'm happy to reopen it if anyone sees the need. @pboutes thank you for your contribution, and so sorry it took this long for that functionality to be live.

[ianfixes]

Whoops, replied to the wrong window somehow -- that message about config overrides was for an unrelated issue. This can be closed.