Lets' revert this change please. I don't think this change pays for itself; the bot already runs godeps from a *clean* working copy so there is no danger of it commiting using the wrong deps and Tim has identified the make targets for developers to check their own local working copy.
On Thu, Aug 21, 2014 at 10:44 AM, Nate Finch <nate.fi...@canonical.com> wrote: > Hmm, no, you're right... Godeps will probably report your dev branch. Hmph. > > On Aug 20, 2014 8:42 PM, "Nate Finch" <nate.fi...@canonical.com> wrote: >> >> You shouldn't have a failing test, you just need to update >> dependencies.tsv to 've correct before running tests... Which you'd need to >> do anyway because you changed the deps. >> >> I don't know why Ian's godeps is broken... >> >> On Aug 20, 2014 8:37 PM, "Tim Penhey" <tim.pen...@canonical.com> wrote: >>> >>> A while back I added some useful make targets: >>> >>> if the env var `JUJU_MAKE_GODEPS` is "true", then >>> >>> make godeps >>> >>> runs the godeps -u, and also makes sure it is run before build and check. >>> >>> I tend to use this. >>> >>> Also, if you are working on integrating changes in other branches that >>> aren't merged, you'll always have a failing test now, no? >>> >>> Tim >>> >>> On 21/08/14 12:33, Ian Booth wrote: >>> > Ah yes godeps -t ./... fails the same way. >>> > But I never run that. >>> > >>> > I just run godeps -u dependencies.tsv >>> > to update the revs of the imported libs as I switch juju-core branches. >>> > >>> > >>> > On 21/08/14 10:30, Nate Finch wrote: >>> >> And running godeps -t ./... Didn't give the same error? >>> >> On Aug 20, 2014 8:26 PM, "Ian Booth" <ian.bo...@canonical.com> wrote: >>> >> >>> >>> Don't think so >>> >>> >>> >>> ian@wallyworld:~$ which godeps >>> >>> /home/ian/juju/go/bin/godeps >>> >>> >>> >>> ian@wallyworld:~$ ls -l `which godeps` >>> >>> -rwxrwxr-x 1 ian ian 4821160 Aug 15 19:12 >>> >>> /home/ian/juju/go/bin/godeps >>> >>> >>> >>> >>> >>> >>> >>> On 21/08/14 10:20, Nate Finch wrote: >>> >>>> All the test does is run godeps -t ./... It looks for godeps in the >>> >>>> path >>> >>>> first and then looks in gopath/bin. Do you maybe have a version in >>> >>>> your >>> >>>> path that is old? >>> >>>> On Aug 20, 2014 8:15 PM, "Ian Booth" <ian.bo...@canonical.com> >>> >>>> wrote: >>> >>>> >>> >>>>> I run godeps all the time as I switch between 1.20 and master. It >>> >>>>> Just >>> >>>>> Works. >>> >>>>> >>> >>>>> On 21/08/14 10:12, Nate Finch wrote: >>> >>>>>> What happens when you run godeps normally? >>> >>>>>> >>> >>>>>> It should ignore the std lib stuff, but I don't actually know how >>> >>> that's >>> >>>>>> implemented. >>> >>>>>> On Aug 20, 2014 8:02 PM, "Ian Booth" <ian.bo...@canonical.com> >>> >>>>>> wrote: >>> >>>>>> >>> >>>>>>> Hmmmm. The test fails for me. >>> >>>>>>> >>> >>>>>>> >>> >>>>>>> FAIL: dependencies_test.go:42: dependenciesTest.TestGodepsIsRight >>> >>>>>>> >>> >>>>>>> dependencies_test.go:77: >>> >>>>>>> ... >>> >>>>>>> dependencies_test.go:70: >>> >>>>>>> c.Fatal(string(out)) >>> >>>>>>> ... Error: godeps: no version control system found for >>> >>>>>>> "/usr/lib/go/src/pkg/bufio" >>> >>>>>>> godeps: no version control system found for >>> >>> "/usr/lib/go/src/pkg/bytes" >>> >>>>>>> godeps: no version control system found for >>> >>> "/usr/lib/go/src/pkg/errors" >>> >>>>>>> godeps: no version control system found for >>> >>>>>>> "/usr/lib/go/src/pkg/io" >>> >>>>>>> godeps: no version control system found for >>> >>>>>>> "/usr/lib/go/src/pkg/sync" >>> >>>>>>> godeps: no version control system found for >>> >>>>>>> "/usr/lib/go/src/pkg/sync/atomic" >>> >>>>>>> ... >>> >>>>>>> ... >>> >>>>>>> >>> >>>>>>> >>> >>>>>>> On 21/08/14 05:18, Nate Finch wrote: >>> >>>>>>>> In an effort to make sure that dependencies.tsv is actually >>> >>>>>>>> correct, >>> >>>>> the >>> >>>>>>>> tests in github.com/juju/juju now run godeps -t ./... and >>> >>>>>>>> compare >>> >>> its >>> >>>>>>>> output to dependencies.tsv. The test looks for godeps first in >>> >>>>>>>> the >>> >>>>>>> $PATH, >>> >>>>>>>> and then in the first $GOPATH/bin. If it is not found, it >>> >>>>>>>> currently >>> >>>>>>> skips >>> >>>>>>>> the test (because CI doesn't have godeps when it runs the >>> >>>>>>>> tests). >>> >>>>>>>> Eventually I'd like to make the test not skippable, since >>> >>>>>>>> anyone >>> >>>>> running >>> >>>>>>>> the tests (except CI) needs godeps anyway... but to get this in >>> >>> sooner >>> >>>>>>>> rather than later, I think it's ok to let it be skipped. >>> >>>>>>>> >>> >>>>>>>> Note that in submitting the PR, I actually had to fix >>> >>> dependencies.tsv >>> >>>>> to >>> >>>>>>>> get the test to pass, because it was wrong.... which is exactly >>> >>>>>>>> why >>> >>>>> this >>> >>>>>>>> test exists. >>> >>>>>>>> >>> >>>>>>>> You can now run go test from github.com/juju/juju and it'll tell >>> >>>>>>>> you >>> >>>>> if >>> >>>>>>>> something's wonky with dependencies.tsv... it'll tell you if >>> >>>>>>>> there >>> >>> are >>> >>>>>>>> entries in it that aren't needed, it'll tell you if you're >>> >>>>>>>> missing >>> >>>>>>> entries >>> >>>>>>>> that should be in it, and it'll tell you if one of your branches >>> >>>>>>>> is >>> >>> on >>> >>>>>>> the >>> >>>>>>>> wrong commit. >>> >>>>>>>> >>> >>>>>>>> *Note:* this means you can't just willy-nilly append your latest >>> >>>>>>> dependency >>> >>>>>>>> to dependencies.tsv. They need to be in alphabetical order. >>> >>>>>>>> The >>> >>>>> easiest >>> >>>>>>>> way to do that is just to run godeps -t ./... > dependencies.tsv >>> >>>>>>>> from >>> >>>>>>>> github.com/juju/juju. That'll produce the right output. >>> >>>>>>>> Obviously, >>> >>>>>>> check >>> >>>>>>>> the diff to make sure the changes it produces are the ones you >>> >>> expect. >>> >>>>>>>> >>> >>>>>>>> This also means that we need to ensure godeps produces the right >>> >>> output >>> >>>>>>> on >>> >>>>>>>> all OSes. If you have a dependency that exists only in an >>> >>> OS-specific >>> >>>>>>>> file, the godeps test will fail on OSes other than that one >>> >>>>>>>> (e.g. if >>> >>>>>>>> there's a dependency in an _windows.go file, the test will fail >>> >>>>>>>> on >>> >>>>> linux, >>> >>>>>>>> because godeps doesn't see the windows dependencies). To fix >>> >>>>>>>> this, >>> >>> the >>> >>>>>>>> easiest thing to do is import the same library in an OS-agnostic >>> >>> file, >>> >>>>>>> and >>> >>>>>>>> give it a name of underscore, which will prevent the unused >>> >>>>>>>> package >>> >>>>>>> error, >>> >>>>>>>> thusly: _ "foo.com/my/win/lib" >>> >>>>>>>> >>> >>>>>>>> -Nate >>> >>>>>>>> >>> >>>>>>>> >>> >>>>>>>> >>> >>>>>>> >>> >>>>>>> -- >>> >>>>>>> Juju-dev mailing list >>> >>>>>>> Juju-dev@lists.ubuntu.com >>> >>>>>>> Modify settings or unsubscribe at: >>> >>>>>>> https://lists.ubuntu.com/mailman/listinfo/juju-dev >>> >>>>>>> >>> >>>>>> >>> >>>>> >>> >>>> >>> >>> >>> >> >>> > >>> > > -- > Juju-dev mailing list > Juju-dev@lists.ubuntu.com > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/juju-dev > -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev