Re: Resolving the same windows test failures again

2015-05-04 Thread Andrew Wilkins 
On Tue, May 5, 2015 at 7:20 AM, Martin Packman  wrote:

> There was some confusion about the regression to the windows test
> failures on trunk.
>
> 
>
> 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:
>
> 
>
> 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:
>
> 
>
> 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:
>
> 
>
> Earliest 1.24 with windows test failures:
>
> 
>
> $ git diff fffe3e4f..95e7619f
>
> 2770 lines... Not helpful. But, error mentions "cannot move the charm
> archive" and:
>
> -gopkg.in/juju/charm.v5 git
> 6b74a2771545912f8a91a544b0f28405b99386242015-04-14T14:33:47Z
> +gopkg.in/juju/charm.v5 git
> 779394167ac61b02ca73ca17c3012f05a5ba316c2015-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:
> 
> 
>

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
> 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

Re: bugs, fixes and targeting Juju versions

2015-05-04 Thread Ian Booth 
Yes, cheery pick is something I use all the time, as it fills out the PR in the
latter branches with a nice commit message based on the original and also
includes the original PR from which the commit was first done.

On 05/05/15 11:45, Jesse Meek wrote:
> Ah, even better. Now I can update my workflow :)
> 
> On 05/05/15 13:43, Menno Smits wrote:
>> cherry-pick will even grab the top commit of a branch if you give the branch
>> name (presuming the fix is a single commit). For example:
>>
>> git checkout -b bug-fix-1.24 upstream/1.24 # create a branch for the fix in 
>> 1.24
>> git cherry-pick bug-fix-master-branch   # pull the fix across
>>
>> There are various ways of grabbing multiple revisions too.
>>
>> And of course, as per Ian's recent email you should be targeting fixes to the
>> lowest affected version and working forwards. So really in your example the
>> fix should be made for 1.24 and the cherry picked onto a branch made from 
>> master.
>>
>>
>>
>>
>> On 5 May 2015 at 13:15, Tim Penhey > > wrote:
>>
>> git cherry-pick does this as a git command.
>>
>> Tim
>>
>>
>> On 05/05/15 13:03, Jesse Meek wrote:
>> > Hi All,
>> >
>> > tl;dr `git diff --no-prefix master > diff.patch; patch -p0 <
>> diff.patch`
>> > is useful for landing bug fixes in different versions of juju.
>> >
>> > As a lot of us are currently bug hunting and needing to land
>> fixes in
>> > multiple versions of Juju, I thought I'd share my process of
>> doing that
>> > (maybe it's helpful?):
>> >
>> > So say you've branched master, let's call it
>> "bug-fix-master-branch",
>> > it's got your fix but you need to land it in 1.24. So branch
>> 1.24, let's
>> > call it bug-fix-124, and do the following:
>> >
>> > # generate a diff of your changes that can be used with patch
>> > (--no-prefix master is the magic flag that generates the right
>> format)
>> > (bug-fix-master-branch) $ git diff --no-prefix master > diff.patch
>> >
>> > # don't add or commit, checkout the other branch
>> > (bug-fix-master-branch) $ git checkout bug-fix-124
>> >
>> > # diff.patch is still there, unstaged. So use it to add the patch
>> > (bug-fix-124) $ patch -p0 < diff.patch
>> >
>> > # do a sanity check
>> > (bug-fix-124) $ git diff
>> >
>> > # remove the patch file
>> > (bug-fix-124) $ rm diff.patch
>> >
>> > You've now got a bug-fix branch eligible for automatic merging
>> targeting
>> > 1.24.
>> >
>> > Cheers,
>> > Jess
>> >
>>
>>
>> --
>> 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

Re: Resolving the same windows test failures again

2015-05-04 Thread Nate Finch 
Sorry for the lack of tests for that part of the code, I guess I added them
for one and not the other.   Note that I had actually been leaving that PR
unmerged because I knew master was blocked.  Sorry it got pushed in anyway,
and caused problems.

On Mon, May 4, 2015 at 7:20 PM, Martin Packman  wrote:

> There was some confusion about the regression to the windows test
> failures on trunk.
>
> 
>
> 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:
>
> 
>
> 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:
>
> 
>
> 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:
>
> 
>
> Earliest 1.24 with windows test failures:
>
> 
>
> $ git diff fffe3e4f..95e7619f
>
> 2770 lines... Not helpful. But, error mentions "cannot move the charm
> archive" and:
>
> -gopkg.in/juju/charm.v5 git
> 6b74a2771545912f8a91a544b0f28405b99386242015-04-14T14:33:47Z
> +gopkg.in/juju/charm.v5 git
> 779394167ac61b02ca73ca17c3012f05a5ba316c2015-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:
> 
> 
>
> 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
> 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

Re: bugs, fixes and targeting Juju versions

2015-05-04 Thread Jesse Meek 

Ah, even better. Now I can update my workflow :)

On 05/05/15 13:43, Menno Smits wrote:
cherry-pick will even grab the top commit of a branch if you give the 
branch name (presuming the fix is a single commit). For example:


git checkout -b bug-fix-1.24 upstream/1.24 # create a branch for the 
fix in 1.24

git cherry-pick bug-fix-master-branch   # pull the fix across

There are various ways of grabbing multiple revisions too.

And of course, as per Ian's recent email you should be targeting fixes 
to the lowest affected version and working forwards. So really in your 
example the fix should be made for 1.24 and the cherry picked onto a 
branch made from master.





On 5 May 2015 at 13:15, Tim Penhey > wrote:


git cherry-pick does this as a git command.

Tim


On 05/05/15 13:03, Jesse Meek wrote:
> Hi All,
>
> tl;dr `git diff --no-prefix master > diff.patch; patch -p0 <
diff.patch`
> is useful for landing bug fixes in different versions of juju.
>
> As a lot of us are currently bug hunting and needing to land
fixes in
> multiple versions of Juju, I thought I'd share my process of
doing that
> (maybe it's helpful?):
>
> So say you've branched master, let's call it
"bug-fix-master-branch",
> it's got your fix but you need to land it in 1.24. So branch
1.24, let's
> call it bug-fix-124, and do the following:
>
> # generate a diff of your changes that can be used with patch
> (--no-prefix master is the magic flag that generates the right
format)
> (bug-fix-master-branch) $ git diff --no-prefix master > diff.patch
>
> # don't add or commit, checkout the other branch
> (bug-fix-master-branch) $ git checkout bug-fix-124
>
> # diff.patch is still there, unstaged. So use it to add the patch
> (bug-fix-124) $ patch -p0 < diff.patch
>
> # do a sanity check
> (bug-fix-124) $ git diff
>
> # remove the patch file
> (bug-fix-124) $ rm diff.patch
>
> You've now got a bug-fix branch eligible for automatic merging
targeting
> 1.24.
>
> Cheers,
> Jess
>


--
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

Re: bugs, fixes and targeting Juju versions

2015-05-04 Thread Menno Smits 
 cherry-pick will even grab the top commit of a branch if you give the
branch name (presuming the fix is a single commit). For example:

git checkout -b bug-fix-1.24 upstream/1.24   # create a branch for the fix
in 1.24
git cherry-pick bug-fix-master-branch   # pull the fix across

There are various ways of grabbing multiple revisions too.

And of course, as per Ian's recent email you should be targeting fixes to
the lowest affected version and working forwards. So really in your example
the fix should be made for 1.24 and the cherry picked onto a branch made
from master.




On 5 May 2015 at 13:15, Tim Penhey  wrote:

> git cherry-pick does this as a git command.
>
> Tim
>
>
> On 05/05/15 13:03, Jesse Meek wrote:
> > Hi All,
> >
> > tl;dr `git diff --no-prefix master > diff.patch; patch -p0 < diff.patch`
> > is useful for landing bug fixes in different versions of juju.
> >
> > As a lot of us are currently bug hunting and needing to land fixes in
> > multiple versions of Juju, I thought I'd share my process of doing that
> > (maybe it's helpful?):
> >
> > So say you've branched master, let's call it "bug-fix-master-branch",
> > it's got your fix but you need to land it in 1.24. So branch 1.24, let's
> > call it bug-fix-124, and do the following:
> >
> > # generate a diff of your changes that can be used with patch
> > (--no-prefix master is the magic flag that generates the right format)
> > (bug-fix-master-branch) $ git diff --no-prefix master > diff.patch
> >
> > # don't add or commit, checkout the other branch
> > (bug-fix-master-branch) $ git checkout bug-fix-124
> >
> > # diff.patch is still there, unstaged. So use it to add the patch
> > (bug-fix-124) $ patch -p0 < diff.patch
> >
> > # do a sanity check
> > (bug-fix-124) $ git diff
> >
> > # remove the patch file
> > (bug-fix-124) $ rm diff.patch
> >
> > You've now got a bug-fix branch eligible for automatic merging targeting
> > 1.24.
> >
> > Cheers,
> > Jess
> >
>
>
> --
> 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

Re: bugs, fixes and targeting Juju versions

2015-05-04 Thread Tim Penhey 
git cherry-pick does this as a git command.

Tim


On 05/05/15 13:03, Jesse Meek wrote:
> Hi All,
> 
> tl;dr `git diff --no-prefix master > diff.patch; patch -p0 < diff.patch`
> is useful for landing bug fixes in different versions of juju.
> 
> As a lot of us are currently bug hunting and needing to land fixes in
> multiple versions of Juju, I thought I'd share my process of doing that
> (maybe it's helpful?):
> 
> So say you've branched master, let's call it "bug-fix-master-branch",
> it's got your fix but you need to land it in 1.24. So branch 1.24, let's
> call it bug-fix-124, and do the following:
> 
> # generate a diff of your changes that can be used with patch
> (--no-prefix master is the magic flag that generates the right format)
> (bug-fix-master-branch) $ git diff --no-prefix master > diff.patch
> 
> # don't add or commit, checkout the other branch
> (bug-fix-master-branch) $ git checkout bug-fix-124
> 
> # diff.patch is still there, unstaged. So use it to add the patch
> (bug-fix-124) $ patch -p0 < diff.patch
> 
> # do a sanity check
> (bug-fix-124) $ git diff
> 
> # remove the patch file
> (bug-fix-124) $ rm diff.patch
> 
> You've now got a bug-fix branch eligible for automatic merging targeting
> 1.24.
> 
> Cheers,
> Jess
> 


-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev

bugs, fixes and targeting Juju versions

2015-05-04 Thread Jesse Meek 

Hi All,

tl;dr `git diff --no-prefix master > diff.patch; patch -p0 < diff.patch` 
is useful for landing bug fixes in different versions of juju.


As a lot of us are currently bug hunting and needing to land fixes in 
multiple versions of Juju, I thought I'd share my process of doing that 
(maybe it's helpful?):


So say you've branched master, let's call it "bug-fix-master-branch", 
it's got your fix but you need to land it in 1.24. So branch 1.24, let's 
call it bug-fix-124, and do the following:


# generate a diff of your changes that can be used with patch 
(--no-prefix master is the magic flag that generates the right format)

(bug-fix-master-branch) $ git diff --no-prefix master > diff.patch

# don't add or commit, checkout the other branch
(bug-fix-master-branch) $ git checkout bug-fix-124

# diff.patch is still there, unstaged. So use it to add the patch
(bug-fix-124) $ patch -p0 < diff.patch

# do a sanity check
(bug-fix-124) $ git diff

# remove the patch file
(bug-fix-124) $ rm diff.patch

You've now got a bug-fix branch eligible for automatic merging targeting 
1.24.


Cheers,
Jess

--
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev

Resolving the same windows test failures again

2015-05-04 Thread Martin Packman 
There was some confusion about the regression to the windows test
failures on trunk.



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:



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:



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:



Earliest 1.24 with windows test failures:



$ git diff fffe3e4f..95e7619f

2770 lines... Not helpful. But, error mentions "cannot move the charm
archive" and:

-gopkg.in/juju/charm.v5 git
6b74a2771545912f8a91a544b0f28405b99386242015-04-14T14:33:47Z
+gopkg.in/juju/charm.v5 git
779394167ac61b02ca73ca17c3012f05a5ba316c2015-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:



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
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev

Re: Do not land code on blocked branches

2015-05-04 Thread John Meinel 
So I believe you can already switch to how the Juju client does it today
(call AddCharm before you call ServiceDeploy), just that we can't require
you to do so.

John
=:->

On Mon, May 4, 2015 at 7:56 PM, Richard Harding 
wrote:

> Thanks for the update. I've closed the bug for now and we'll reopen an
> issue to update to the latest api version once that's changed.
>
> On Sun, 03 May 2015, John Meinel wrote:
>
> > Just to follow up one small point, Rick. The reason Juju has to fix the
> API
> > and restore the old behavior is because even if Quickstart fixed itself,
> > anyone running an older version is going to break with a newer Juju API,
> > which is why we require API stability, and only changing things by
> bumping
> > the version. :)
> >
> > John
> > =:->
>
-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev

Re: Do not land code on blocked branches

2015-05-04 Thread Richard Harding 
Thanks for the update. I've closed the bug for now and we'll reopen an
issue to update to the latest api version once that's changed.

On Sun, 03 May 2015, John Meinel wrote:

> Just to follow up one small point, Rick. The reason Juju has to fix the API
> and restore the old behavior is because even if Quickstart fixed itself,
> anyone running an older version is going to break with a newer Juju API,
> which is why we require API stability, and only changing things by bumping
> the version. :)
> 
> John
> =:->

-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev