Disable collision-protect in parallel_emerge.

Disable collision-protect in parallel_emerge.

collision-protect doesn't make sense for parallel_emerge, because
we don't lock around merges. Also, if merge fails part-way through,
it's strange to fail just because some files are lying around.
This fixes an issue with the buildbots.

TEST=Checked that collision-protect feature gets deleted by this.
BUG=none

[zbehan, 2010-07-29 07:59:22]

LGTM, thanks!

On Thu, Jul 29, 2010 at 12:54 AM, <davidjames@chromium.org> wrote:

> Reviewers: zbehan,
>
> Description:
> Disable collision-protect in parallel_emerge.
>
> collision-protect doesn't make sense for parallel_emerge, because
> we don't lock around merges. Also, if merge fails part-way through,
> it's strange to fail just because some files are lying around.
> This fixes an issue with the buildbots.
>
> TEST=Checked that collision-protect feature gets deleted by this.
> BUG=none
>
> Please review this at http://codereview.chromium.org/3061029/show
>
> SVN Base: ssh://git@chromiumos-git/crosutils.git
>
> Affected files:
>  M parallel_emerge
>
>
> Index: parallel_emerge
> diff --git a/parallel_emerge b/parallel_emerge
> index
>
12548fa878444c88c91de6667fd710dce7622f00..58c2e963da537c34a5469b020969490c654e3324
> 100755
> --- a/parallel_emerge
> +++ b/parallel_emerge
> @@ -356,6 +356,9 @@ class DepGraphGenerator(object):
>     if (settings.get("PORTAGE_DEBUG", "") == "1" and
>         "python-trace" in settings.features):
>       portage.debug.set_trace(True)
> +    # Since we don't have locking around merges, the collision-protect
> +    # feature doesn't make sense.
> +    settings.features.discard("collision-protect")
>
>     # Complain about unsupported options
>     for opt in ("--ask", "--ask-enter-invalid", "--complete-graph",
>
>
>

[Mandeep Singh Baines, 2010-07-29 13:12:18]

davidjames@chromium.org (davidjames@chromium.org) wrote:
> Reviewers: zbehan,
>
> Description:
> Disable collision-protect in parallel_emerge.
>
> collision-protect doesn't make sense for parallel_emerge, because
> we don't lock around merges. Also, if merge fails part-way through,
> it's strange to fail just because some files are lying around.

I'm not following. Shouldn't each file be owned by exactly one package.

Won't this mask bugs. If two packages are owning a file, we won't
notice it after this CL. This will result in non-determinstic build
depending on the order in which packages are merged.

> This fixes an issue with the buildbots.
>
> TEST=Checked that collision-protect feature gets deleted by this.
> BUG=none
>
> Please review this at http://codereview.chromium.org/3061029/show
>
> SVN Base: ssh://git@chromiumos-git/crosutils.git
>
> Affected files:
>   M parallel_emerge
>
>
> Index: parallel_emerge
> diff --git a/parallel_emerge b/parallel_emerge
> index  
>
12548fa878444c88c91de6667fd710dce7622f00..58c2e963da537c34a5469b020969490c654e3324

> 100755
> --- a/parallel_emerge
> +++ b/parallel_emerge
> @@ -356,6 +356,9 @@ class DepGraphGenerator(object):
>      if (settings.get("PORTAGE_DEBUG", "") == "1" and
>          "python-trace" in settings.features):
>        portage.debug.set_trace(True)
> +    # Since we don't have locking around merges, the collision-protect
> +    # feature doesn't make sense.
> +    settings.features.discard("collision-protect")
>
>      # Complain about unsupported options
>      for opt in ("--ask", "--ask-enter-invalid", "--complete-graph",
>
>

[davidjames, 2010-07-29 15:04:52]

On 2010/07/29 13:12:18, Mandeep Singh Baines wrote:
> mailto:davidjames@chromium.org (mailto:davidjames@chromium.org) wrote:
> > Reviewers: zbehan,
> >
> > Description:
> > Disable collision-protect in parallel_emerge.
> >
> > collision-protect doesn't make sense for parallel_emerge, because
> > we don't lock around merges. Also, if merge fails part-way through,
> > it's strange to fail just because some files are lying around.
> 
> I'm not following. Shouldn't each file be owned by exactly one package.
> 
> Won't this mask bugs. If two packages are owning a file, we won't
> notice it after this CL. This will result in non-determinstic build
> depending on the order in which packages are merged.

Sorry, maybe I should have been more clear in the description.

Portage has two flags for doing collision protection: collision-protect and
protect-owned. The protect-owned feature is enabled by default and is quite
useful -- it checks to make sure that we don't have multiple packages that own
the same file. The collision-protect feature goes beyond protect-owned and also
protects files that aren't owned by any package. E.g. if you merge a package and
then CTRL-C in the middle of the merge, and then merge again, the merge fails
the second time if you have collision-protect, because collision-protect spotted
that the old merge left some files around that don't belong to any package, and
maybe you don't want to replace them. Unlike collision-protect, protect-owned
does not cause problems here because it only protects files that actually belong
to a real package.

[Mandeep Singh Baines, 2010-07-29 16:40:17]

davidjames@chromium.org (davidjames@chromium.org) wrote:
> On 2010/07/29 13:12:18, Mandeep Singh Baines wrote:
> >mailto:davidjames@chromium.org (mailto:davidjames@chromium.org) wrote:
> >> Reviewers: zbehan,
> >>
> >> Description:
> >> Disable collision-protect in parallel_emerge.
> >>
> >> collision-protect doesn't make sense for parallel_emerge, because
> >> we don't lock around merges. Also, if merge fails part-way through,
> >> it's strange to fail just because some files are lying around.
> 
> >I'm not following. Shouldn't each file be owned by exactly one package.
> 
> >Won't this mask bugs. If two packages are owning a file, we won't
> >notice it after this CL. This will result in non-determinstic build
> >depending on the order in which packages are merged.
> 
> Sorry, maybe I should have been more clear in the description.
> 
> Portage has two flags for doing collision protection: collision-protect and
> protect-owned. The protect-owned feature is enabled by default and is quite
> useful -- it checks to make sure that we don't have multiple
> packages that own
> the same file. The collision-protect feature goes beyond
> protect-owned and also
> protects files that aren't owned by any package. E.g. if you merge a
> package and
> then CTRL-C in the middle of the merge, and then merge again, the
> merge fails
> the second time if you have collision-protect, because
> collision-protect spotted
> that the old merge left some files around that don't belong to any
> package, and
> maybe you don't want to replace them. Unlike collision-protect,
> protect-owned
> does not cause problems here because it only protects files that
> actually belong
> to a real package.
> 

Ah. Thanks for the explanation:)

> http://codereview.chromium.org/3061029/show