On Tue, May 5, 2015 at 7:20 AM, Martin Packman <[email protected] > wrote:
> There was some confusion about the regression to the windows test > failures on trunk. > > <https://bugs.launchpad.net/juju-core/+bug/1450919> > > Partly my fault, Curtis initially looked at the 1.24 branch and I > looked at trunk, and each branch has a different issue. Here's what > I've just done to diagnose. > > > So, trunk first, I noticed the failures looked exactly the same as > before CI fiddled with the environment variable casing to work around > a juju bug: > > <https://launchpad.net/bugs/1446871> > > The fix for this bug landed on master, therefore it's easy to assume > that change actually broke the casing behaviour in such a way as to > invalidate the CI hack, rather than fixing the underlying issue. > > Looking at the code: > > <https://github.com/juju/juju/pull/2124/files> > > Two similar functions now exist for merging case, mergeEnvWin which > works on map[string]string and mergeWindowsEnvironment which works on > []string. Only mergeEnvWin has tests. Guess, mergeWindowsEnvironment > is bugged. Indeed: > > - m[strings.ToUpper(varSplit[0])] = varSplit[1] > + k := varSplit[0] > + if _, ok := uppers[strings.ToUpper(k)]; ok { > + uppers[k] = varSplit[1] > > It's not uppercasing the key in the assignment. So, Path is matched to > PATH, Path is assigned to, but later only PATH is pulled out. > > > Now for the 1.24 branch, no hints here. So, lets see what changed. > Latest working 1.24 without windows test failures: > > <http://reports.vapour.ws/releases/2581> > > Earliest 1.24 with windows test failures: > > <http://reports.vapour.ws/releases/2592> > > $ git diff fffe3e4f..95e7619f > > 2770 lines... Not helpful. But, error mentions "cannot move the charm > archive" and: > > -gopkg.in/juju/charm.v5 git > 6b74a2771545912f8a91a544b0f28405b9938624 2015-04-14T14:33:47Z > +gopkg.in/juju/charm.v5 git > 779394167ac61b02ca73ca17c3012f05a5ba316c 2015-04-30T02:46:55Z > > $ pushd ~/go/src/gopkg.in/juju/charm.v5 > > Try to update and branches diverged? Wha? > > $ git diff 6b74a277..77939416 > > Er... this includes a revert of my licence header changes, and, the > answer to the test failures, Gabriel's file closing fix: > <https://github.com/juju/charm/pull/119> > <https://github.com/juju/charm/pull/120> > Those fixes were not made on v5, and only existed on v6-unstable. I later came along and merged some changes into v5 and updated Juju's dependencies.tsv to the latest commit on v5. That meant the fixes were dropped. I've cherry-picked the fixes into v5, and have a merge job queued for https://github.com/juju/juju/pull/2212. Please, everyone, be careful to ensure fixes go on the appropriate branch. For gopkg.in-based packages, that means only pinning to commits on the branch related to the package path suffix. Cheers, Andrew So, just renaming a variable is *not* safe (if you accidentally back > out other changes when merging). > > > Both these regressions happened due to landing 'safe' changes while > the branch was broken, and were therefore harder to pin down than they > would otherwise have been. > > Martin > > -- > Juju-dev mailing list > [email protected] > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/juju-dev >
-- Juju-dev mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
