<div dir="ltr">The main problem with putting code into a different directory is that if there are files in that repo that reference the package itself, they'd need to be updated.<div><br></div><div>So like for example, if flag_test.go imports "<a href="http://launchpad.net/gnuflag">launchpad.net/gnuflag</a>" to do external tests, we'd need to edit that code to import "gnuflag" instead</div><div><br></div><div>Also, if there are any third party packages that use one of the same dependencies we do, we'd need to edit those files, too... </div><div><div><br><div class="gmail_quote"><div dir="ltr">On Fri, Aug 12, 2016 at 2:31 PM Nicholas Skaggs <<a href="mailto:nicholas.skaggs@canonical.com">nicholas.skaggs@canonical.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Aug 12, 2016 at 9:11 AM, Nate Finch <span dir="ltr"><<a href="mailto:nate.finch@canonical.com" target="_blank">nate.finch@canonical.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>The problem really stems from two go-isms - </div><div><br></div><div>1.) The standard for Go projects is to have the path on disk representative of the URL to the project.</div><div>2.) Every file that uses a package has an import statement that references the path on disk.</div><div><br></div><div>If either one of these were not true, we could have avoided your problem.</div><div><br></div><div>There's nothing we can do about #2. Building with the go tool requires that if you use a package in a file, you must have an import statement for it, and if you import "foo/bar/gnuflag", that code must live in $GOPATH/src/foo/bar/gnuflag". </div><div><br></div><div>For #1, <i>in theory</i> we could change things. Only "go get" knows that $GOPATH/src/<a href="http://launchpad.net/gnuflag" target="_blank">launchpad.net/gnuflag</a> corresponds to <a href="http://launchpad.net/gnuflag" target="_blank">http://launchpad.net/gnuflag</a>. When you compile and build, all the go tool cares about is the path on disk and what code is in that folder. We <i>could</i> detach the idea of an import path from the URL it corresponds to. We could git clone <a href="http://github.com/juju/gnuflag" target="_blank">github.com/juju/gnuflag</a> into $GOPATH/src/gnuflag, and the go tool would build it just fine. the import statement would then be import "gnuflag" and this would work just fine. We'd need to maintain a mapping file of repo to folder on disk, similar to what we do now with dependencies.tsv, but it wouldn't be the end of the world.</div><div><br></div><div>I'm not entirely convinced this is a good idea, but it would make updating dependencies a lot easier.</div></div></blockquote></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>This is interesting, and more or less what I was after in my mail. I wasn't sure if there were any out of the box ideas that would make life easier, but I hoped to surface some for discussion. Thanks!</div><div><br></div><div>I suppose my follow-up question is, why or why not is this a good idea? Having to change all of the imports is painful, but as you say, it is an intentional design decision by the Go Community. I don't have any real feeling or inkling on what other Go projects do, but I suspect at least some of them follow this idea of importing all dependencies and updating them only if needed. However, by nature of them simply cloning the source into the repo, I would also guess they are not able to easily update a dependency.</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>The source code hack to mgo is due to this problem as well. In theory we can just fork mgo and make our fix, but that would again require us to change a lot of import statements.</div><div><br></div><div>Part of the problem was that you were changing two very common dependencies that were imported 4 levels deep (i.e. a imports b imports c imports d, and they all import the dependency). This wouldn't change no matter what we do - you'd still need to start the changes at d and roll them up the stack. That would be true in any language.</div></div></blockquote></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Yes, I told of my pain, but as painful as it was, I got lots of help from many of you. Thank you again for helping land this. </div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div><span style="line-height:1.5">The "circular" dependency (in quotes because you can't have a true circular dependency in go, but you can have two repos that both depend on each other) shouldn't have landed that way. </span><span style="line-height:1.5">Two repos should definitely not depend on each other. </span><span style="line-height:1.5"> No other repo should ever depend on <a href="http://github.com/juju/juju" target="_blank">github.com/juju/juju</a>. We explicitly make no claims for backwards compatibility, so the thing you're depending on could just disappear one day. If you need something in there, factor it out into its own repo with a backwards compatibility guarantee first.</span></div><div><br></div><div>It *is* a problem that dependencies under juju can be imported in broken states because of our use of dependencies.tsv. There's nothing that says we couldn't run all the unit tests for all packages juju uses in the landing job for <a href="http://github.com/juju/juju" target="_blank">github.com/juju/juju</a> (and any other repo we desire)... just run go test ./... from $GOPATH/src and it'll test everything we have downloaded to build juju with. That's an easy win, and something we should start doing ASAP in my opinion.</div></div></blockquote></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>This was my initial takeaway. We don't have testing in the subpackages when landing, and we should. This I agree should be straightforward enough to fix. I was curious if there were other low hanging fruit like this.</div><div><br></div><div>The other oddity was keeping common dependencies in sync. That is to say, if juju/utils needs version 2016-03-05, juju/juju can request the same depends, but use version 2015-12-10. They don't have to be the same, and in reality aren't. This makes sense as they are separate projects. It would seem unless someone (like me) needs to update something, they don't. However, this ends up causing you to cascade dependency updates you didn't plan on when you attempt to make a change. There might not be anything to change here.</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div><br></div><div>Finally... a lot of the reasons these things hurt so much is because juju is a huge project - over 500,000 lines of code. What is a minor annoyance in a small project becomes much worse in a large project. Obviously, part of that is inescapable - we can't make juju smaller. I do think it's worth considering ways to make our lives easier.</div></div><span><font color="#888888"><div><br></div><div>-Nate</div></font></span></div><div><div><br><div class="gmail_quote"><div dir="ltr">On Thu, Aug 11, 2016 at 7:08 PM Casey Marshall <<a href="mailto:casey.marshall@canonical.com" target="_blank">casey.marshall@canonical.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Aug 11, 2016 at 5:44 PM, Nicholas Skaggs <span dir="ltr"><<a href="mailto:nicholas.skaggs@canonical.com" target="_blank">nicholas.skaggs@canonical.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">This is a simple story of a man and a simple mission. Eliminate the final 2 dependencies that are in bazaar and launchpad. It makes juju and it's dependencies live completely in git. A notable goal, and one that I desired for getting snaps to build with launchpad.<div><br></div><div>I don't feel I need to explain the pain that making a no-source change update to a dependency (point juju and friends to it's new location) has been. I've had to carefully craft a set of PR's, and land them in a certain order. I've encountered contention (because I have to change hundreds of imports), unit test issues (because juju's dependencies aren't tested when merged, so they can be incompatible with the latest juju without knowing it), and circular dependencies that require some magic and wishful thinking to workaround.</div><div><br></div><div>I'm still not finished landing the change, but I hope to do so *soon*. It must be close now!</div><div><br></div><div>All of this to say, I think it would be useful to have a discussion on how we manage dependencies within the project. From a release perspective, it can be quite cumbersome as dependencies are picked up and dropped. It's also recently made a release (And critical bugfix) really difficult at times. From my newly experience contributor perspective, I would really think twice before attempting to make an update :-) I suspect I'm not alone.</div><div><br></div><div>I've heard ideas in the past about cleaning this up, and some things like circular dependencies between romulus and juju are probably best described as tech debt. But there also is some pain in the larger scheme of things. For example, we are currently hacking a patch to juju's source for the mgo dependency since updating the source or vendoring or any other option is way too painful. It's time to really fix this. Ideas?</div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>My team's been chipping away at romulus and it'll be sorted out soon enough. We've already moved the terms API client out to <a href="http://github.com/juju/terms-client" target="_blank">github.com/juju/terms-client</a>, and we'll be doing something similar for the other APIs. As for the commands.. these probably need to find a better home closer to the command base types they extend, in cmd/juju/...</div><div><br></div><div>One thing that occurred to me today though is most of our dependencies also have tests (well, they should!). We don't often run <i>those</i> tests as part of Juju CI, but you could run into some cases where some dependencies share common dependencies, but are tested with different common dependency versions than those specified by Juju's dependencies.tsv.</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>--<br>
Juju-dev mailing list<br>
<a href="mailto:Juju-dev@lists.ubuntu.com" target="_blank">Juju-dev@lists.ubuntu.com</a><br>
Modify settings or unsubscribe at: <a href="https://lists.ubuntu.com/mailman/listinfo/juju-dev" rel="noreferrer" target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju-dev</a><br>
<br></blockquote></div></div></div>
--<br>
Juju-dev mailing list<br>
<a href="mailto:Juju-dev@lists.ubuntu.com" target="_blank">Juju-dev@lists.ubuntu.com</a><br>
Modify settings or unsubscribe at: <a href="https://lists.ubuntu.com/mailman/listinfo/juju-dev" rel="noreferrer" target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju-dev</a><br>
</blockquote></div>
</div></div></blockquote></div></div></div></blockquote></div></div></div></div>