Change git-rebase-update to always keep branch 'master'.

Change git-rebase-update to always keep branch 'master'.

It's useful to keep a clean branch locally. It's annoying when it's constantly
removed.

Fix pylint errors, print_func one was valid since the argument was ignored.

R=iannucci@chromium.org
BUG=

[M-A Ruel, 2014-11-25 20:38:31]

https://codereview.chromium.org/756223002/diff/1/gclient_scm.py
File gclient_scm.py (right):

https://codereview.chromium.org/756223002/diff/1/gclient_scm.py#newcode301
gclient_scm.py:301: filter_fn=GitDiffFilterer(self.relpath,
print_func=self.Print).Filter)
I can only assume this code path is not used.

Probably worth deleting this.

https://codereview.chromium.org/756223002/diff/1/tests/git_rebase_update_test.py
File tests/git_rebase_update_test.py (right):

https://codereview.chromium.org/756223002/diff/1/tests/git_rebase_update_test...
tests/git_rebase_update_test.py:123: self.repo.git('checkout', '-b', 'master',
'origin/master')
This doesn't add coverage for the 2 new lines, not sure why.

[iannucci, 2014-11-25 21:05:59]

What is master useful for? Why not just check out origin/master?

On Tue, Nov 25, 2014, 12:38 null <maruel@chromium.org> wrote:

> Reviewers: iannucci,
>
>
> https://codereview.chromium.org/756223002/diff/1/gclient_scm.py
> File gclient_scm.py (right):
>
> https://codereview.chromium.org/756223002/diff/1/gclient_scm.py#newcode301
> gclient_scm.py:301: filter_fn=GitDiffFilterer(self.relpath,
> print_func=self.Print).Filter)
> I can only assume this code path is not used.
>
> Probably worth deleting this.
>
> https://codereview.chromium.org/756223002/diff/1/tests/
> git_rebase_update_test.py
> File tests/git_rebase_update_test.py (right):
>
> https://codereview.chromium.org/756223002/diff/1/tests/
> git_rebase_update_test.py#newcode123
> tests/git_rebase_update_test.py:123
>
<https://codereview.chromium.org/756223002/diff/1/tests/git_rebase_update_test...>:
> self.repo.git('checkout', '-b',
> 'master', 'origin/master')
> This doesn't add coverage for the 2 new lines, not sure why.
>
> Description:
> Change git-rebase-update to always keep branch 'master'.
>
> It's useful to keep a clean branch locally. It's annoying when it's
> constantly
> removed.
>
> Fix pylint errors, print_func one was valid since the argument was ignored.
>
> R=iannucci@chromium.org
> BUG=
>
> Please review this at https://codereview.chromium.org/756223002/
>
> Base URL: svn://svn.chromium.org/chrome/trunk/tools/depot_tools
>
> Affected files (+6, -3 lines):
>    M fix_encoding.py
>    M gclient_scm.py
>    M git_rebase_update.py
>    M tests/git_rebase_update_test.py
>
>
> Index: fix_encoding.py
> diff --git a/fix_encoding.py b/fix_encoding.py
> index
> 61bd742b495043b97a653a4fd6fda40eca087a52..5da9135818d4c3cb71d09c7dd02441
> 340205aaf1
> 100644
> --- a/fix_encoding.py
> +++ b/fix_encoding.py
> @@ -229,7 +229,7 @@ class WinUnicodeConsoleOutput(WinUnicodeOutputBase):
>           remaining -= n.value
>           if not remaining:
>             break
> -        text = text[n.value:]
> +        text = text[int(n.value):]
>       except Exception, e:
>         complain('%s.write: %r' % (self.name, e))
>         raise
> Index: gclient_scm.py
> diff --git a/gclient_scm.py b/gclient_scm.py
> index
> 95c142f93c8954fab67b72a2e623dedc986824de..e42f6716387fb720a263392290191d
> 9ea7f56456
> 100644
> --- a/gclient_scm.py
> +++ b/gclient_scm.py
> @@ -298,7 +298,7 @@ class GitWrapper(SCMWrapper):
>       gclient_utils.CheckCallAndFilter(
>           ['git', 'diff', merge_base],
>           cwd=self.checkout_path,
> -        filter_fn=GitDiffFilterer(self.relpath).Filter,
> print_func=self.Print)
> +        filter_fn=GitDiffFilterer(self.relpath,
> print_func=self.Print).Filter)
>
>     def _FetchAndReset(self, revision, file_list, options):
>       """Equivalent to git fetch; git reset."""
> @@ -1188,7 +1188,7 @@ class SVNWrapper(SCMWrapper):
>           ['svn', 'diff', '-x', '--ignore-eol-style'] + args,
>           cwd=self.checkout_path,
>           print_stdout=False,
> -        filter_fn=SvnDiffFilterer(self.relpath).Filter,
> print_func=self.Print)
> +        filter_fn=SvnDiffFilterer(self.relpath,
> print_func=self.Print).Filter)
>
>     def update(self, options, args, file_list):
>       """Runs svn to update or transparently checkout the working copy.
> Index: git_rebase_update.py
> diff --git a/git_rebase_update.py b/git_rebase_update.py
> index
> a61e53a3ab5b5b94e482e0eef9e71fd82ad9f2c6..5ad0ee176fd2d1296bed485d723f55
> b961a3ef15
> 100755
> --- a/git_rebase_update.py
> +++ b/git_rebase_update.py
> @@ -84,6 +84,8 @@ def remove_empty_branches(branch_tree):
>     deletions = set()
>     downstreams = collections.defaultdict(list)
>     for branch, parent in git.topo_iter(branch_tree, top_down=False):
> +    if branch == 'master':
> +      continue
>       downstreams[parent].append(branch)
>
>       if git.hash_one(branch) == git.hash_one(parent):
> Index: tests/git_rebase_update_test.py
> diff --git a/tests/git_rebase_update_test.py
> b/tests/git_rebase_update_test.py
> index
> 84e111b587d4a9ac0aac11c7cae056acce3dcaa4..185cc73bdbd79b8760d107ce5779e0
> f6e99d772b
> 100755
> --- a/tests/git_rebase_update_test.py
> +++ b/tests/git_rebase_update_test.py
> @@ -120,6 +120,7 @@ class
> GitRebaseUpdateTest(git_test_utils.GitRepoReadWriteTestBase):
>       self.repo.run(self.nb.main, ['--upstream-current', 'empty_branch2'])
>
>       self.repo.git('checkout', 'branch_K')
> +    self.repo.git('checkout', '-b', 'master', 'origin/master')
>
>       output, _ = self.repo.capture_stdio(self.reup.main)
>
>
>
>

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

[M-A Ruel, 2014-11-25 22:07:14]

On 2014/11/25 21:05:59, iannucci wrote:
> What is master useful for? Why not just check out origin/master?

I have repo foo.git on linux-host
I need to test on Windows, because Windows, so I git clone linux-host:foo
I want to compare result between my branch 'awesome' and 'master'.

Oops, 'master' is not there.

[iannucci, 2014-11-25 22:09:21]

On 2014/11/25 22:07:14, M-A Ruel wrote:
> On 2014/11/25 21:05:59, iannucci wrote:
> > What is master useful for? Why not just check out origin/master?
> 
> I have repo foo.git on linux-host
> I need to test on Windows, because Windows, so I git clone linux-host:foo
> I want to compare result between my branch 'awesome' and 'master'.
> 
> Oops, 'master' is not there.

git upstream-diff awesome ?
git diff awesome origin/master ?

[M-A Ruel, 2014-11-25 22:10:03]

On 2014/11/25 22:09:21, iannucci wrote:
> On 2014/11/25 22:07:14, M-A Ruel wrote:
> > On 2014/11/25 21:05:59, iannucci wrote:
> > > What is master useful for? Why not just check out origin/master?
> > 
> > I have repo foo.git on linux-host
> > I need to test on Windows, because Windows, so I git clone linux-host:foo
> > I want to compare result between my branch 'awesome' and 'master'.
> > 
> > Oops, 'master' is not there.
> 
> git upstream-diff awesome ?
> git diff awesome origin/master ?

You're missing the point, there's no origin/master at all, it was deleted.

[iannucci, 2014-11-25 22:12:00]

On 2014/11/25 22:09:21, iannucci wrote:
> On 2014/11/25 22:07:14, M-A Ruel wrote:
> > On 2014/11/25 21:05:59, iannucci wrote:
> > > What is master useful for? Why not just check out origin/master?
> > 
> > I have repo foo.git on linux-host
> > I need to test on Windows, because Windows, so I git clone linux-host:foo
> > I want to compare result between my branch 'awesome' and 'master'.
> > 
> > Oops, 'master' is not there.
> 
> git upstream-diff awesome ?
> git diff awesome origin/master ?

The reason I'm opposed to this sort of thing is because I'm tired of helping
people try to understand why they have a master branch and an origin/master
branch. The 'master' branch is a copy of origin/master which is usually out of
date, and tends to have people commit stuff to it, which then confuses them
because their commits are on master, but they're not in the upstream!

If we were to implement the feature you're trying to add here, I would request:
  * make it a configuration multi-option `depot-tools.keep-branch`
  * have it blank by default
  * add it to the git-rebase-update manpage.

[iannucci, 2014-11-25 22:12:19]

On 2014/11/25 22:10:03, M-A Ruel wrote:
> On 2014/11/25 22:09:21, iannucci wrote:
> > On 2014/11/25 22:07:14, M-A Ruel wrote:
> > > On 2014/11/25 21:05:59, iannucci wrote:
> > > > What is master useful for? Why not just check out origin/master?
> > > 
> > > I have repo foo.git on linux-host
> > > I need to test on Windows, because Windows, so I git clone linux-host:foo
> > > I want to compare result between my branch 'awesome' and 'master'.
> > > 
> > > Oops, 'master' is not there.
> > 
> > git upstream-diff awesome ?
> > git diff awesome origin/master ?
> 
> You're missing the point, there's no origin/master at all, it was deleted.

origin/master is never deleted...

[iannucci, 2014-11-25 22:12:44]

On 2014/11/25 22:12:19, iannucci wrote:
> On 2014/11/25 22:10:03, M-A Ruel wrote:
> > On 2014/11/25 22:09:21, iannucci wrote:
> > > On 2014/11/25 22:07:14, M-A Ruel wrote:
> > > > On 2014/11/25 21:05:59, iannucci wrote:
> > > > > What is master useful for? Why not just check out origin/master?
> > > > 
> > > > I have repo foo.git on linux-host
> > > > I need to test on Windows, because Windows, so I git clone
linux-host:foo
> > > > I want to compare result between my branch 'awesome' and 'master'.
> > > > 
> > > > Oops, 'master' is not there.
> > > 
> > > git upstream-diff awesome ?
> > > git diff awesome origin/master ?
> > 
> > You're missing the point, there's no origin/master at all, it was deleted.
> 
> origin/master is never deleted...

or, if it's deleted, it's because you deleted the remote, not because
git-rebase-update deleted it.

[M-A Ruel, 2014-11-26 14:17:16]

On 2014/11/25 22:12:44, iannucci wrote:
> On 2014/11/25 22:12:19, iannucci wrote:
> > On 2014/11/25 22:10:03, M-A Ruel wrote:
> > > You're missing the point, there's no origin/master at all, it was deleted.
> > 
> > origin/master is never deleted...
> 
> or, if it's deleted, it's because you deleted the remote, not because
> git-rebase-update deleted it.

You still misread what I wrote. The origin remote *is* linux-host on my OSX and
Windows hosts. I need to ensure my code works on multiple platforms, so I use my
linux host *as the origin*. Your blanket assumption that origin points to
chromium.googlesource, github or code.google.com is wrong. :)


> The reason I'm opposed to this sort of thing is because I'm tired of helping
> people try to understand why they have a master branch and an origin/master
> branch. The 'master' branch is a copy of origin/master which is usually out of
> date, and tends to have people commit stuff to it, which then confuses them
> because their commits are on master, but they're not in the upstream!

Ok this is a convincing argument. I wish we could trust devs to learn the tools
they use day to day. Sadly, I agree that's an incorrect assumption.


> If we were to implement the feature you're trying to add here, I would
request:
>   * make it a configuration multi-option `depot-tools.keep-branch`
>   * have it blank by default
>   * add it to the git-rebase-update manpage.

That's a good idea. I may eventually send a new patchset to this CL to do that.