Added packages/infra_libs as the first Python package.

Added packages/infra_libs as the first Python package.

Zero code change, just copying the files.

BUG=631098
R=agable@chromium.org

Committed: https://chromium.googlesource.com/infra/infra/+/a95f0d91b339c2370913a6728729bd9af3fd8d51

[Sergey Berezin, 2016-07-25 17:07:35]

PTAL

[Sergey Berezin, 2016-07-25 17:07:56]

The CQ bit was checked by sergeyberezin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-25 17:07:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-25 17:23:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-25 17:23:29]

Dry run: Try jobs failed on following builders:
  Infra Win Tester on master.tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Infra%20Win%20Tester/bu...)

[Sergey Berezin, 2016-07-25 19:07:14]

The CQ bit was checked by sergeyberezin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-25 19:07:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-25 19:21:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-25 19:21:49]

Dry run: This issue passed the CQ dry run.

[Sergey Berezin, 2016-07-27 18:18:57]

ping?

[Sergey Berezin, 2016-07-28 00:03:04]

sergeyberezin@chromium.org changed reviewers:
+ iannucci@chromium.org

[Sergey Berezin, 2016-07-28 00:03:04]

+iannucci - thanks!

[Sergey Berezin, 2016-07-30 00:49:49]

Ping? Pretty-please?

[agable, 2016-08-01 16:54:33]

I guess... if there are zero code changes, why is it being put in /packages/ at
all? Why can't we package it directly from infra.git/infra_libs?

[Sergey Berezin, 2016-08-01 18:35:36]

On 2016/08/01 16:54:33, agable wrote:
> I guess... if there are zero code changes, why is it being put in /packages/
at
> all? Why can't we package it directly from infra.git/infra_libs?

Because we need a deeper directory structure to create valid packages, and I'd
like to allow for more than one package in infra.git:

packages/
  package_name1/
     setup.py
     package_module1/
     package_module2/
  package_name2/
    etc...

Otherwise there is no easy way to write a valid setup.py for each package.

In particular, there is no good way to preserve the import paths for infra_libs
once it turns into a package in its current location. What I'm trying to do
instead is create a package first, get it bootstrapped into infra.git, and then
I can delete the original infra_libs while preserving all the import paths
everywhere.

[agable, 2016-08-01 18:39:41]

On 2016/08/01 at 18:35:36, sergeyberezin wrote:
> On 2016/08/01 16:54:33, agable wrote:
> > I guess... if there are zero code changes, why is it being put in /packages/
at
> > all? Why can't we package it directly from infra.git/infra_libs?
> 
> Because we need a deeper directory structure to create valid packages, and I'd
like to allow for more than one package in infra.git:
> 
> packages/
>   package_name1/
>      setup.py
>      package_module1/
>      package_module2/
>   package_name2/
>     etc...
> 
> Otherwise there is no easy way to write a valid setup.py for each package.
> 
> In particular, there is no good way to preserve the import paths for
infra_libs once it turns into a package in its current location. What I'm trying
to do instead is create a package first, get it bootstrapped into infra.git, and
then I can delete the original infra_libs while preserving all the import paths
everywhere.

I think if that's the plan, you should have the full set of CLs and processes
for that ready to go before this lands. I think the amount of time in which
there are two copies of infra_libs checked in to the same repository should be
on the order of minutes, not days while the rest of the packaging and vendoring
gets set up.

[Sergey Berezin, 2016-08-01 19:16:04]

On 2016/08/01 18:39:41, agable wrote:

> I think if that's the plan, you should have the full set of CLs and processes
> for that ready to go before this lands. I think the amount of time in which
> there are two copies of infra_libs checked in to the same repository should be
> on the order of minutes, not days while the rest of the packaging and
vendoring
> gets set up.

I don't think that's necessary, or in fact possible. My plan is to keep
//packages out of PYTHONPATH, so they will not interfere with the existing
infra_libs initially. I'll then update gsubtreed config to mirror infra_libs
from //packages/infra_libs. Once it propagates through gsubtreed, I'll prepare a
single CL to install infra_libs as a package into ENV, and delete infra_libs
from its original location. Since this switch is atomic, there should be no
confusion for the imports.

Note that I can't have this last CL until I have a commit hash for the gsubtreed
repo, which requires a package to be in infra.git.

[agable, 2016-08-01 20:16:18]

On 2016/08/01 at 19:16:04, sergeyberezin wrote:
> On 2016/08/01 18:39:41, agable wrote:
> 
> > I think if that's the plan, you should have the full set of CLs and
processes
> > for that ready to go before this lands. I think the amount of time in which
> > there are two copies of infra_libs checked in to the same repository should
be
> > on the order of minutes, not days while the rest of the packaging and
vendoring
> > gets set up.
> 
> I don't think that's necessary, or in fact possible. My plan is to keep
//packages out of PYTHONPATH, so they will not interfere with the existing
infra_libs initially. I'll then update gsubtreed config to mirror infra_libs
from //packages/infra_libs. Once it propagates through gsubtreed, I'll prepare a
single CL to install infra_libs as a package into ENV, and delete infra_libs
from its original location. Since this switch is atomic, there should be no
confusion for the imports.
> 
> Note that I can't have this last CL until I have a commit hash for the
gsubtreed repo, which requires a package to be in infra.git.

I'm not concerned about imports, I'm concerned about changes having to be made
in two places, or the migration accidentally reverting changes which were only
made in one place.

You can create an empty directory, set up gsubtreed to mirror it, and write a
doc outlining the exact set of commands you'll run (and/or CLs you'll land) to
switch infra to using the new package. Then two commits in relatively rapid
succession: one to add the files to packages/, and one to replace infra_libs
with the new package in ENV. The only amount of time you should have to wait
between those two CLs is the amount of time it takes for gsubtreed to propagate
the change.

[iannucci, 2016-08-01 20:45:42]

On 2016/08/01 20:16:18, agable wrote:
> On 2016/08/01 at 19:16:04, sergeyberezin wrote:
> > On 2016/08/01 18:39:41, agable wrote:
> > 
> > > I think if that's the plan, you should have the full set of CLs and
> processes
> > > for that ready to go before this lands. I think the amount of time in
which
> > > there are two copies of infra_libs checked in to the same repository
should
> be
> > > on the order of minutes, not days while the rest of the packaging and
> vendoring
> > > gets set up.
> > 
> > I don't think that's necessary, or in fact possible. My plan is to keep
> //packages out of PYTHONPATH, so they will not interfere with the existing
> infra_libs initially. I'll then update gsubtreed config to mirror infra_libs
> from //packages/infra_libs. Once it propagates through gsubtreed, I'll prepare
a
> single CL to install infra_libs as a package into ENV, and delete infra_libs
> from its original location. Since this switch is atomic, there should be no
> confusion for the imports.
> > 
> > Note that I can't have this last CL until I have a commit hash for the
> gsubtreed repo, which requires a package to be in infra.git.
> 
> I'm not concerned about imports, I'm concerned about changes having to be made
> in two places, or the migration accidentally reverting changes which were only
> made in one place.
> 
> You can create an empty directory, set up gsubtreed to mirror it, and write a
> doc outlining the exact set of commands you'll run (and/or CLs you'll land) to
> switch infra to using the new package. Then two commits in relatively rapid
> succession: one to add the files to packages/, and one to replace infra_libs
> with the new package in ENV. The only amount of time you should have to wait
> between those two CLs is the amount of time it takes for gsubtreed to
propagate
> the change.

(I think agable is much closer to the python ecosystem in infra.git, so I'll
defer to his review here)

[Sergey Berezin, 2016-08-01 21:31:23]

On 2016/08/01 20:16:18, agable wrote:
> I'm not concerned about imports, I'm concerned about changes having to be made
> in two places, or the migration accidentally reverting changes which were only
> made in one place.
> 
> You can create an empty directory, set up gsubtreed to mirror it, and write a
> doc outlining the exact set of commands you'll run (and/or CLs you'll land) to
> switch infra to using the new package. Then two commits in relatively rapid
> succession: one to add the files to packages/, and one to replace infra_libs
> with the new package in ENV. The only amount of time you should have to wait
> between those two CLs is the amount of time it takes for gsubtreed to
propagate
> the change.

Ah, good point, thanks. I'll start with an empty //packages/infra_libs then.
Given that the rate of change is pretty low in infra_libs (less than one commit
a day), I'll do the following:

* Create an empty //packages/infra_libs, and make sure gsubtreed correctly
mirrors it
* Set up noparent in infra_libs/OWNERS and make myself the only owner - to
acquire a mutex lock on any changes
* Land essentially this CL, making sure all the original changes are picked up.
* Once it's mirrored, land the CL to install infra_libs in ENV and delete the
original infra_libs.

[Sergey Berezin, 2016-08-02 21:59:58]

sergeyberezin@chromium.org changed reviewers:
- iannucci@chromium.org

[Sergey Berezin, 2016-08-02 21:59:59]

-iannucci (saving the spam)

This is set up as a dependent CL, and is ready to roll.

[agable, 2016-08-03 16:36:20]

lgtm

[Sergey Berezin, 2016-08-03 19:42:01]

The CQ bit was checked by sergeyberezin@chromium.org

[Sergey Berezin, 2016-08-03 19:42:01]

The patchset sent to the CQ was uploaded after l-g-t-m from agable@chromium.org
Link to the patchset: https://codereview.chromium.org/2179853002/#ps80001
(title: "Final rebase (no actual changes)")

[commit-bot: I haz the power, 2016-08-03 19:42:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-03 19:57:23]

Description was changed from

==========
Added packages/infra_libs as the first Python package.

Zero code change, just copying the files.

BUG=631098
R=agable@chromium.org
==========

to

==========
Added packages/infra_libs as the first Python package.

Zero code change, just copying the files.

BUG=631098
R=agable@chromium.org

Committed:
https://chromium.googlesource.com/infra/infra/+/a95f0d91b339c2370913a6728729b...
==========

[commit-bot: I haz the power, 2016-08-03 19:57:24]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/infra/infra/+/a95f0d91b339c2370913a6728729b...