Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(78)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 192323006: Revert of Another attempt: gclient: delete mismatching checkouts (Closed)

Created:
6 years, 9 months ago by Nico
Modified:
6 years, 9 months ago
CC:
chromium-reviews, Dirk Pranke, cmp-cc_chromium.org, M-A Ruel, iannucci+depot_tools_chromium.org, ilevy-cc_chromium.org, skiabot_google.com
Visibility:
Public.

Description

Revert of Another attempt: gclient: delete mismatching checkouts (https://codereview.chromium.org/183283003/) Reason for revert: Broke `gclient sync` for me, failing with: Error: 1> Can't update/checkout /Volumes/MacintoshHD2/src/chrome-git/src if an unversioned directory is present. Delete the directory and try again. For someone else, it broke it with: % gclient sync ________ unmanaged solution; skipping src Error: Command svn info --xml returned non-zero exit status 1 in /Users/pawliger/chromium/src/. <?xml version="1.0" encoding="UTF-8"?> <info> svn: E155007: '/Users/pawliger/chromium/src' is not a working copy Original issue's description: > Another attempt: gclient: delete mismatching checkouts > > Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=255930 TBR=iannucci@chromium.org,szager@chromium.org,maruel@chromium.org,mmoss@chromium.org,borenet@google.com NOTREECHECKS=true NOTRY=true Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=256005

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+41 lines, -425 lines) Patch
M gclient_scm.py View 7 chunks +19 lines, -35 lines 0 comments Download
M scm.py View 1 chunk +1 line, -1 line 0 comments Download
M testing_support/fake_repos.py View 1 chunk +0 lines, -76 lines 0 comments Download
M tests/gclient_scm_test.py View 12 chunks +20 lines, -134 lines 0 comments Download
M tests/gclient_smoketest.py View 2 chunks +1 line, -167 lines 0 comments Download
M tests/scm_unittest.py View 1 chunk +0 lines, -12 lines 0 comments Download

Messages

Total messages: 22 (0 generated)
Nico
Created Revert of Another attempt: gclient: delete mismatching checkouts
6 years, 9 months ago (2014-03-10 18:13:19 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/thakis@chromium.org/192323006/1
6 years, 9 months ago (2014-03-10 18:13:53 UTC) #2
borenet
Can you post the contents of your .gclient file? I'd really rather fix the problem ...
6 years, 9 months ago (2014-03-10 18:20:00 UTC) #3
Nico
Since gclient is kinda important, it's probably going to be both. My .gclient: solutions = ...
6 years, 9 months ago (2014-03-10 18:22:39 UTC) #4
borenet
On 2014/03/10 18:22:39, Nico wrote: > Since gclient is kinda important, it's probably going to ...
6 years, 9 months ago (2014-03-10 18:27:57 UTC) #5
Nico
On Mon, Mar 10, 2014 at 11:27 AM, <borenet@google.com> wrote: > On 2014/03/10 18:22:39, Nico ...
6 years, 9 months ago (2014-03-10 18:29:39 UTC) #6
borenet
On 2014/03/10 18:29:39, Nico wrote: > On Mon, Mar 10, 2014 at 11:27 AM, <mailto:borenet@google.com> ...
6 years, 9 months ago (2014-03-10 18:30:30 UTC) #7
Nico
On Mon, Mar 10, 2014 at 11:30 AM, <borenet@google.com> wrote: > On 2014/03/10 18:29:39, Nico ...
6 years, 9 months ago (2014-03-10 18:31:13 UTC) #8
borenet
On 2014/03/10 18:31:13, Nico wrote: > On Mon, Mar 10, 2014 at 11:30 AM, <mailto:borenet@google.com> ...
6 years, 9 months ago (2014-03-10 18:41:54 UTC) #9
Nico
On Mon, Mar 10, 2014 at 11:41 AM, <borenet@google.com> wrote: > On 2014/03/10 18:31:13, Nico ...
6 years, 9 months ago (2014-03-10 18:46:00 UTC) #10
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-10 19:01:04 UTC) #11
commit-bot: I haz the power
Presubmit check for 192323006-1 failed and returned exit status -2001. The presubmit check was hung. ...
6 years, 9 months ago (2014-03-10 19:01:05 UTC) #12
Nico
Here's the other gclient file: solutions = [ { 'managed': False, 'name': 'src', 'url': 'https://src.chromium.org/chrome/trunk/src', ...
6 years, 9 months ago (2014-03-10 19:27:55 UTC) #13
Nico
The CQ bit was checked by thakis@chromium.org
6 years, 9 months ago (2014-03-10 19:28:10 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/thakis@chromium.org/192323006/1
6 years, 9 months ago (2014-03-10 19:28:16 UTC) #15
borenet
On 2014/03/10 18:46:00, Nico wrote: > On Mon, Mar 10, 2014 at 11:41 AM, <mailto:borenet@google.com> ...
6 years, 9 months ago (2014-03-10 19:29:34 UTC) #16
commit-bot: I haz the power
Change committed as 256005
6 years, 9 months ago (2014-03-10 19:30:23 UTC) #17
Nico
On Mon, Mar 10, 2014 at 12:29 PM, <borenet@google.com> wrote: > On 2014/03/10 18:46:00, Nico ...
6 years, 9 months ago (2014-03-10 19:43:37 UTC) #18
iannucci
So, if your .gclient claims that src is an svn repo, but it's a really ...
6 years, 9 months ago (2014-03-10 20:10:40 UTC) #19
Nico
On Mon, Mar 10, 2014 at 1:10 PM, Robert Iannucci <iannucci@chromium.org>wrote: > So, if your ...
6 years, 9 months ago (2014-03-10 20:13:07 UTC) #20
borenet
On 2014/03/10 20:13:07, Nico wrote: > On Mon, Mar 10, 2014 at 1:10 PM, Robert ...
6 years, 9 months ago (2014-03-10 20:19:01 UTC) #21
iannucci
6 years, 9 months ago (2014-03-10 20:19:21 UTC) #22 
Okay, fair enough. Gclient seems to actually have an endless number of
configurations which were all 'correct' at one point or another, which
makes gclient borderline impossible to evolve :/

I think all you'll need to do is change your 'url' to '
https://chromium.googlesource.com/chromium/src.git', and set 'managed' to
False.  This will clue gclient in explicitly to take the Git scm path, and
then not to touch src at all.

This should be the case for both .gclient files, unless I'm missing
something.

In general, Eric, WDYT about detecting this case and auto-migrating old
.gclient's? Or at least printing an error with instructions on how to
correct when we detect this scenario? Actually... auto-correcting could be
dangerous.


On Mon, Mar 10, 2014 at 1:13 PM, Nico Weber <thakis@chromium.org> wrote:

> On Mon, Mar 10, 2014 at 1:10 PM, Robert Iannucci
<iannucci@chromium.org>wrote:
>
>> So, if your .gclient claims that src is an svn repo, but it's a really a
>> git repo, then your .gclient is already broken. The fact that gclient
>> happens to still work is a testament to the lack of error checking in
>> gclient, not because it's WAI :)
>>
>
> No, that's how old git worked. Evan explicitly added code for this to
> gclient long ago.
>
> (I'm happy to add unmanaged to my .gclient if this is the only problem. Is
> the failure from the other gclient file understood?)
>
>
>>
>> That said, there are very compelling reasons to land this change.
>> Specifically, the DEPS should be respected as the source of truth, and
>> right now gclient does not correctly handle DEPS which switch SCMs. This
>> change is intended to fix that.
>>
>>
>> On Mon, Mar 10, 2014 at 12:43 PM, Nico Weber <thakis@chromium.org> wrote:
>>
>>> On Mon, Mar 10, 2014 at 12:29 PM, <borenet@google.com> wrote:
>>>
>>>> On 2014/03/10 18:46:00, Nico wrote:
>>>>
>>>>  On Mon, Mar 10, 2014 at 11:41 AM, <mailto:borenet@google.com> wrote:
>>>>>
>>>>
>>>>  > On 2014/03/10 18:31:13, Nico wrote:
>>>>> >
>>>>> >  On Mon, Mar 10, 2014 at 11:30 AM, <mailto:borenet@google.com>
>>>>> wrote:
>>>>> >>
>>>>> >
>>>>> >  > On 2014/03/10 18:29:39, Nico wrote:
>>>>> >> >
>>>>> >> >  On Mon, Mar 10, 2014 at 11:27 AM, <mailto:borenet@google.com>
>>>>> wrote:
>>>>> >> >>
>>>>> >> >
>>>>> >> >  > On 2014/03/10 18:22:39, Nico wrote:
>>>>> >> >> >
>>>>> >> >> >> Since gclient is kinda important, it's probably going to be
>>>>> both.
>>>>> >> >> >>
>>>>> >> >> >
>>>>> >> >> >  My .gclient:
>>>>> >> >> >>
>>>>> >> >> >
>>>>> >> >> >  solutions = [
>>>>> >> >> >>    { "name"        : "src",
>>>>> >> >> >>      "url"         : "svn://svn.chromium.org/chrome/trunk/src
>>>>> ",
>>>>> >> >> >>      "custom_deps" : {
>>>>> >> >> >>        "src/third_party/WebKit": None,
>>>>> >> >> >>        "src/third_party/WebKit/LayoutTests": None,
>>>>> >> >> >>        "src/third_party/WebKit/Source": None,
>>>>> >> >> >>        "src/third_party/WebKit/Tools/DumpRenderTree": None,
>>>>> >> >> >>        "src/third_party/WebKit/Tools/Scripts": None,
>>>>> >> >> >>        "src/third_party/WebKit/Tools/TestWebKitAPI": None,
>>>>> >> >> >>        "src/third_party/WebKit/WebKitLibraries": None,
>>>>> >> >> >>        "src/rlz": None,
>>>>> >> >> >>      },
>>>>> >> >> >>      "custom_vars" : {
>>>>> >> >> >>        'webkit_trunk':'svn://svn.chromium.org/blink/trunk',
>>>>> >> >> >>      },
>>>>> >> >> >>      "safesync_url": ""
>>>>> >> >> >>    },
>>>>> >> >> >> ]
>>>>> >> >> >>
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >  I'll send you the gclient file from the other person once I
>>>>> have it.
>>>>> >> >> >>
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> > And what do you get when you run "svn info ." inside src?
>>>>> >> >>
>>>>> >> >
>>>>> >> >
>>>>> >> >  svn: '.' is not a working copy
>>>>> >> >>
>>>>> >> >
>>>>> >> >  I'm pulling src from git. gclient used to say "________ found
>>>>> .git
>>>>> >> >> directory; skipping src" before your change.
>>>>> >> >>
>>>>> >> >
>>>>> >> >
>>>>> >> >
>>>>> >> > In that case, shouldn't this be an unmanaged checkout?
>>>>> >>
>>>>> >
>>>>> >
>>>>> >  All the deps still come from svn (it's an "old git" checkout).
>>>>> >>
>>>>> >
>>>>> >
>>>>> >
>>>>> > I think that's okay - the key is that if you aren't expecting
>>>>> gclient to
>>>>> > update
>>>>> > the chrome checkout, it should be set to unmanaged.
>>>>>
>>>>
>>>>
>>>>  1. This checkout has worked fine as-is for 4 years or so since 2010. It
>>>>> should continue working unless there's a good reason for it not to.
>>>>> 2. The change broke someone else's checkout too.
>>>>>
>>>>
>>>>
>>>>
>>>> Here's some background: Skia wants badly to fully switch to git.
>>>>  Currently,
>>>> chromium uses three partial checkouts of Skia, which makes it
>>>> impossible for us
>>>> to switch until we can replace src/third_party/skia with a full
>>>> checkout of our
>>>> git repository.  There were a few ways of going about it, but a number
>>>> of us met
>>>> with chrome-infra and decided that the cleanest way to do it was to
>>>> modify
>>>> gclient so that local checkouts which didn't match the DEPS entries
>>>> were deleted
>>>> (with --force) or threw an error (without --force).  This is a new
>>>> behavior for
>>>> gclient, which previously just ignored git checkouts when given an svn
>>>> url.  But
>>>> it allows us to create a DEPS change like this one:
>>>> https://codereview.chromium.org/192743005/ and the bots will happily
>>>> delete
>>>> third_party/skia and re-sync.  It will require some manual action from
>>>> developers who don't use --force, but we thought that was okay assuming
>>>> it was
>>>> paired with a warning ("You'll need to delete third_party/skia").
>>>>
>>>> If you think this is the wrong way to do it, I'm happy to schedule
>>>> another
>>>> meeting to find a solution which we can agree on.  Personally, I think
>>>> this is
>>>> much better than adding something skia-specific to gclient, and much,
>>>> much
>>>> better than just changing the DEPS and letting the bots fail and check
>>>> out from
>>>> scratch, which I'm told has a high probability of bringing down svn
>>>> mirrors.
>>>
>>>
>>> I don't have any background on this change. I noticed that `gclient
>>> sync` was broken when I came in this morning, and that someone had sent me
>>> a "help, `gclient sync` is broken!" email too. Since this was caused from a
>>> change that landed a few hours ago and it was monday early-ish on the west
>>> coast, I figured I'd revert before lots of others run into this.
>>>
>>> I'm happy with whatever happens, as long as it doesn't break the
>>> existing setups of developers.
>>>
>>>
>>>>
>>>>
>>>>  >
>>>>> >
>>>>> >
>>>>> >  >
>>>>> >> >
>>>>> >> >  >  If it's anything
>>>>> >> >> > other than "svn://svn.chromium.org/chrome/trunk/src", then
>>>>> that's
>>>>> >> the
>>>>> >> >> > problem,
>>>>> >> >> > and it's solved by fixing the .gclient file to match.
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >  On Mon, Mar 10, 2014 at 11:20 AM, <mailto:borenet@google.com>
>>>>> >> wrote:
>>>>> >> >> >>
>>>>> >> >> >
>>>>> >> >> >  > Can you post the contents of your .gclient file?  I'd really
>>>>> >> rather
>>>>> >> >> fix
>>>>> >> >> >> the
>>>>> >> >> >> > problem than continue reverting, as this gets me no closer
>>>>> to
>>>>> >> >> actually
>>>>> >> >> >> > solving
>>>>> >> >> >> > the problem.
>>>>> >> >> >> >
>>>>> >> >> >> > https://codereview.chromium.org/192323006/
>>>>> >> >> >> >
>>>>> >> >> >>
>>>>> >> >> >
>>>>> >> >> >  To unsubscribe from this group and stop receiving emails from
>>>>> it,
>>>>> >> send
>>>>> >> >> an
>>>>> >> >> >>
>>>>> >> >> > email
>>>>> >> >> >
>>>>> >> >> >> to mailto:chromium-reviews+unsubscribe@chromium.org.
>>>>> >> >> >>
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> > https://codereview.chromium.org/192323006/
>>>>> >> >> >
>>>>> >> >>
>>>>> >> >
>>>>> >> >  To unsubscribe from this group and stop receiving emails from
>>>>> it, send
>>>>> >> an
>>>>> >> >>
>>>>> >> > email
>>>>> >> >
>>>>> >> >> to mailto:chromium-reviews+unsubscribe@chromium.org.
>>>>> >> >>
>>>>> >> >
>>>>> >> >
>>>>> >> >
>>>>> >> > https://codereview.chromium.org/192323006/
>>>>> >> >
>>>>> >>
>>>>> >
>>>>> >  To unsubscribe from this group and stop receiving emails from it,
>>>>> send an
>>>>> >>
>>>>> > email
>>>>> >
>>>>> >> to mailto:chromium-reviews+unsubscribe@chromium.org.
>>>>> >>
>>>>> >
>>>>> >
>>>>> >
>>>>> > https://codereview.chromium.org/192323006/
>>>>> >
>>>>>
>>>>
>>>>  To unsubscribe from this group and stop receiving emails from it, send
>>>>> an
>>>>>
>>>> email
>>>>
>>>>> to mailto:chromium-reviews+unsubscribe@chromium.org.
>>>>>
>>>>
>>>>
>>>> https://codereview.chromium.org/192323006/
>>>>
>>>
>>>
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

Powered by Google App Engine
This is Rietveld 408576698