Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.

Workflow for editing dependencies? #1247

Closed
bradleypeabody opened this issue Oct 7, 2017 · 18 comments
Closed

Workflow for editing dependencies? #1247

bradleypeabody opened this issue Oct 7, 2017 · 18 comments

Comments

@bradleypeabody
Copy link

bradleypeabody commented Oct 7, 2017

I'm trying to see if there is any supported workflow (or even a simple workaround) for the situation of needing to make edits in a dependency in a way that still works properly with the dependency's repository.

To walk through an example: package "z" depends on packages "y" and "x". After you've done a dep ensure you end with a folder structure like:

/GOPATH/src/z
/GOPATH/src/z/vendor/x
/GOPATH/src/z/vendor/y

My question is, what happens if I need to make some changes to "y" and I want those changes to be committed and pushed back to "y"'s original repository (on github or wherever)? Is there any supported workflow for that?

My first attempt to address this was to just delete "y" from the vendor folder and "go get" it separately. Annoying, but could work as a temporary hack. But then what happens when "y" depends on "x" (in addition to the fact that "z" depends on "x"). This means that to do edits on "y" I end up with a folder structure like this (with "x" both under the vendor folder and pulled in by "go get"):

/GOPATH/src/z
/GOPATH/src/z/vendor/x
/GOPATH/src/x
/GOPATH/src/y

And to do it required manually deleting /GOPATH/src/z/vendor/y after a dep ensure. Clearly, this is not an intended use of "dep".

Thus my question above - is there a supported way to deal with this?

If not... an idea of a feature that could address this would be a mode (maybe it's a subcommand like "checkout") where instead of checking everything out and then removing the .git folders, it could check everything out and leave them there - so the developer can do whatever manual git (or other VCS) commands they want. Further "dep ensure" commands could even just refuse to operate after this has been done, requiring the developer to manually clean things up and then delete the vendor folder and start over.

--

I realize that a likely response to this is going to be "well, don't do that - you should just edit your dependency directly and it should have its own tests and you should not have to call package y from z in order to know that it works before committing and pushing your changes". My experience is that the world isn't perfect and this feature is needed - especially early on in the life a project when big refactors might be needed. I wouldn't say I run into this issue every day, but I do run into it pretty much every time begin a project that is large enough to warrant multiple repositories and it remains an issue until the project's API and tests are stable enough to allow things to be edited separately without concern.

@kevpie
Copy link

kevpie commented Oct 8, 2017

@bradleypeabody
Glide used to work this way (up to a specific 0.10.x version that I don't recall) . And it was very convenient to make changes to your own forks of libraries or contributing patches. Glide moved towards the caching model that you see now. Before I cutover to Dep I used to keep an old version of Glide around for this purpose.

Cloning into vendor directly would satisfy the pain I'm going through right now with Dep. When trying to add additional dependencies you have to manually track your patches outside of version control.

@carolynvs
Copy link
Collaborator

Yeah, I agree, it's not easy right now to do this! Our readme covers the current options

https://github.com/golang/dep#testing-changes-to-a-dependency

  1. delete from vendor and go get
  2. update the source in your Gopkg.toml to a fork
  3. virtualgo

I don't recommend cloning directly into vendor, it is way too easy to lose all your work that way, since dep will overwrite any changes you've made the next time you run dep ensure.

We've had feature requests for referencing repos on your local filesystem (#935) but that's not happening soon so one of the three choices above are your best bet.

@carolynvs
Copy link
Collaborator

To be clear, we would like to make this better and I'm just closing this because I think we have a few solutions already planned in other issues:

#890
#935

@bradleypeabody
Copy link
Author

bradleypeabody commented Oct 9, 2017

@carolynvs Got it and thanks - having an option to "go get" and then "git checkout" in addition to the current vendor behavior (meaning the same vendor behavior is the normal "dep ensure" but there's a separate mode you can run which causes things to be go gotten and you can run this if you please) - that would be a workable solution.

Is something like that the plan? It wasn't clear to me from those other issues if there was a specific approach that was seriously being considered.

@carolynvs
Copy link
Collaborator

carolynvs commented Oct 11, 2017

Oh, hmm, maybe I misunderstood the workflow you are looking to support. What I was getting at with those two issues is that at some point I'd like for dep to support referencing local repositories on your filesystem.

You could put something like this in your manifest:

[[constraint]]
  name = "carolyn.lovespuppies/andcats"
  source = "file:///home/carolynvs/catsandpuppies.git"

When you are actively working on a dependency, you would go get the dependency to put it in your GOPATH, and then update the manifest in another project to reference the local repo. This would let you use the regular dep ensure workflow without modification, and iterate on the dependency.

It's very similar to the fork workaround suggested in the README, but wouldn't require you to push your changes up to a remote constantly just for local testing.

Is that a workable solution for the original problem?

@ebrady
Copy link

ebrady commented Jan 27, 2018

I'm coming in a bit late here... but am curious about a mode to tell dep to not use vendor/ but instead put the repo under src.. as a full repo (go get), checked out at the correct version as determined by dep (or in the Gopkg.toml file settings). Using the example above, all dependent packages end up here with full and complete git clones (or hg or bzr or whatever):

/GOPATH/src/x
/GOPATH/src/y
/GOPATH/src/z

I think 'dep init' is already good with that, effectively looking at anything you might already have and using those to get versions/etc (if using -gopath). It seems like if 'dep ensure' had the ability to basically go get them and checkout the desired version, then one would be good to go (or, if it's already there, merge to the new version via user defined mechanism). More complexity if the user has local mods and considerations would need to come into play but you could simply fail if that was the case (local mods not stashed stop the cmd before it does anything). If no local mods user could select pull (with or withour rebase mode as desired via config choice)... and it would merge to the next version (unless told not to merge, per config flag/option in gopkg.toml).

This gives me full repo's to work with if I wish... I have the ability to make changes to them and push them up directly in the workspace. Anyone using this mode would be aware that the Gopkg.lock would only map to the original config unless someone ran something that compared the Gopkg.toml with the workspace state, indicated differences, made updates to it and generated a matchin Gopkg.lock to correlated to existing work (and that would fail if things weren't checked in or stashed/etc). Something along those lines. Anyhow, the ability to work with full repo's using standard Go paths would be powerful and is pretty much a requirement for some folks in my org.

Anyhow, this may not be the right issue to add this too but seemed somewhat inline... I guess I'm just not a fan of vendor/ and unversioned dependencies there as versioning grants so much power that tossing it away seems ... tragic, and extremely limiting. Having a mode to work with full repo's would be great. Appreciate any time/thoughts, thanks much!

@sdboyer
Copy link
Member

sdboyer commented Jan 27, 2018

@ebrady the closest thing to what you're talking about is discussed in #935.

@bradleypeabody
Copy link
Author

@ebrady If I understand you correctly you are asking for the same thing I am - and I agree it would be very useful.

@sdboyer Call me thick in the head, but I don't see the relation to issue #935 (probably because I've never used govendor and so I'm missing a lot of context). But the issue here is not trying to add files from the local GOPATH/src to the dep config - it's that we need a way to "go get" the dependencies so they arrive in GOPATH/src (instead of the vendor folder) - with the sole purpose of being able to hack on the dependencies and commit them back to their original repos (or a fork of them or whatever - the point is that they are just a regular git clone and so normal git operations work on them).

Just to recap a real-world scenario: Let's say I make project github.com/bradleypeabody/batman and it depends on github.com/gocraft/dbr for some database stuff. Then I find I need to make some changes to github.com/gocraft/dbr (add a method let's say - and either I admin that repo or I have a fork that I want to point at while the dbr people are looking at my PR) - with dep as it is now, I'm basically screwed - there's no workflow to do that. What would be great is a command which would, instead of checking out github.com/gocraft/dbr into the vendor folder, do the equivalent of a "go get" on all of the dependencies (ideally still pulling the same version from the dep toml files). This would allow me to just edit directly in GOPATH/src/github.com/gocraft/dbr and do what I need to do manually.

I realize that by doing this dep could not provide guarantees about anything after running this command - but that's fine - as long as the developer knows that, it's not an issue and this would still be very very useful.

Does this request make sense?

@sdboyer
Copy link
Member

sdboyer commented Jan 27, 2018

it makes sense, yes, but it ends up in the same place as #935 - just, with the addition of automatically fetching into GOPATH, then allowing builds to occur in a mixed vendor/GOPATH environment (which is #935).

the complexity of interacting with GOPATH, as it is defined today, drastically increases the failure modes and complexity of the tool. it's not that i don't see the use case. i've wanted something like it myself. it's a question of where we spend the limited, unpaid tokens that we have.

FWIW, i'd be happy to guide someone who worked on an external tool that at least takes care of most of this, which...shouldn't be impossible?

@bradleypeabody
Copy link
Author

Okay, I see. So anything that relies on GOPATH is not in the scope of what dep handles, at least as it stands today.

Doing it with an external tool is a definite possibility. I guess the functionality is basically: parse the toml files and come up with a list of package names and versions, and for each one run "go get", and then run "git checkout" to switch to the right version. I'm concerned that the code that is dealing with the configuration would be duplicated and possibly could become brittle as dep evolves, but other than that concern, I could see it work.

If I wrote it would the dep project link to it somewhere and say nice things about me? : D (Joking, but having it linked to somewhere other people could find it would be relevant.)

@sdboyer
Copy link
Member

sdboyer commented Jan 27, 2018

We've exported the types for interacting with the metadata files from the beginning, specifically to facilitate people building atop dep in this way :)

Now, we're pre-1.0.0, which means that the exported API is unstable. But the file parts, at least, are quite stable, so i don't think that should give you much of a problem. Also, i hear there's this dependency management tool out there that can help you with that sort of thing... 😉 👿

And yes, if you write a tool that serves a use case we can't get to, then we'll definitely link to it where appropriate 😄

@bradleypeabody
Copy link
Author

Nice! I will try to look at this closer soon and see if this is something I can put together.

Some immediate questions though just from a quick glance:

  • What about the code that switches to the correct commit/tag? Is that exported somewhere also?
  • And on the Manifest and Lock types, just following the functions and the type names it looks like I would 1) create dep.Ctx and populate it with the right path etc 2) call LoadProject on it, 3) call MakeParams() on the project and then 4) extract the Manifest and Lock from the resulting SolveParameters - does that seem right?

@sdboyer
Copy link
Member

sdboyer commented Jan 27, 2018

What about the code that switches to the correct commit/tag? Is that exported somewhere also?

"switches," like, writes out vendor/? yes, and yes, though the connotation of "switches" suggests that you might benefit from reading this, as it suggests a too-simple view of what's going on.

it looks like I would

Yes, that sounds like roughly the right track.

@bradleypeabody
Copy link
Author

Great and thanks; and I will read that in more detail.

To clarify, what I meant by "switch" was the meaning as used in git - to switch your current branch/tag/commit. I.e. once the particular version of a package we want is determined, something needs to take our version e.g. "1.5.1" and interact with git and do "git checkout v1.5.1" or some similar means of interacting with the version control system to arrive at the correct version of the files. I can and will dive into the doc more, I was just hoping there was a particular struct or two in the godoc that would be the right place to look for that.

(I do realize that there's a bunch of stuff that has to happen to get from the raw data that is in the Manifest and Lock to a specific version number (git tag) available in the repo. But I guess I just need to dive deeper into the guts to wrap my wits around that.)

@sdboyer
Copy link
Member

sdboyer commented Jan 27, 2018

Ah yes, so, that actual git operation is here. But that's not directly exposed - it's arrived at from here, which in turn is called from WriteDepTree(), which was one of the things i linked in my last comment.

@ebrady
Copy link

ebrady commented Jan 28, 2018

Offline yesterday, missed all the excitement.
@sdboyer Thanks much for responding so quickly and with such detail, much appreciated... I appreciate the pointers... that will help with experimentation.
@bradleypeabody Yes, that is what I am looking for as well... although I might take it to an extreme with vendor/ never touched by 'dep' in this mode... ie: it would always bring in full clones for all dependencies it determined were not already covered via any "existing" vendor/ dirs in any of the packages being brought in.

The idea is basically that I always want full clones... as one can do much more with them. I am a bit curious about bringing in a pkg that may have vendored something under itself already... and how that might be honored (especially if some other pkg is dependent upon that same pkg but does not vendor it). If Go can get confused by two different versions of a package in the same Go workspace then that becomes quite a challenge to deal with. Never a dull moment... I'll need to read/catch up on things as I've been a bit focused at work and not keeping up with the 'dep' discussions. The ideal solution there is that Go simply separate those enough so they won't step on each other in any way (separate state/var space/etc). Anyhow, will read up and see what I can find... I too might try and find some time to do something with this.

@gadelkareem
Copy link

For the new Go Module use replace https://github.com/golang/go/wiki/Modules#when-should-i-use-the-replace-directive

@cben
Copy link

cben commented Jan 28, 2019

testing-changes-to-a-dependency moved to the FAQ in #1677 but then you removed it as "incorrect" in 89bca42 (EDIT: #1768 has more details why incorrect).
@sdboyer is there any updated recommendation how to test local changes?

Basically, it's still a frequently asked question, even a stub answer will be appreciated :-)

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

No branches or pull requests

7 participants