[PATCH 0/3] nmbug:

[PATCH 0/3] nmbug:

[W. Trevor King]

Two changes and a bugfix spun off from today's IRC disussion.

We probably also want to bump nmbug's __version__ to 0.3.  Changes
since 0.2, including the patches in this series, ordered by decreasing
impact on 0.2 users:

* Accept failures to unset core.worktree in clone (this series).
* Use --no-renames in log (f9189a06, 2016-09-26, v0.24).
* Auto-checkout in clone if it wouldn't clobber (this series).
* Add a 'help' command for folks who don't like --help (9d25c97d, 2014-10-03, v0.20).
* Setup a 'config' branch on clone to track origin/config (244f8739,
  2015-03-22, v0.20).  This branch may be consumed by notmuch-report(1).
* Ignore # comments in 'notmuch dump ...' output (9bbc54bd, 2016-03-27, v0.22).
* Respect 'expect' in _spawn(..., wait=True) (this series).
* Update URLs in documentation (554b90b5 and 6a833a6e8, 2016-06-02, v0.23).

I expect the best time to make that bump is just before we cut our
next notmuch release.  Once we do, we can update the wiki page [1] to
suggest nmbug 0.3+ and remove the checkout step from “Getting
started”.

Cheers,
Trevor

[1]: https://notmuchmail.org/nmbug/

W. Trevor King (3):
  nmbug: Respect 'expect' in _spawn(..., wait=True)
  nmbug: Accept failures to unset core.worktree in clone
  nmbug: Auto-checkout in clone if it wouldn't clobber

 devel/nmbug/nmbug | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

-- 
2.13.6

[PATCH 1/4] nmbug: Respect 'expect' in _spawn(..., wait=True)

[W. Trevor King]

Fixing a bug from 7f2cb3be (nmbug: Translate to Python, 2014-10-03).
The bug had no direct impact though, because none of the wait=True
callers were setting expect.

Also add expected codes to the debug messages, to help log readers
understand why nonzero exits are occasionally accepted.
---
 devel/nmbug/nmbug | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/devel/nmbug/nmbug b/devel/nmbug/nmbug
index 6febf16f..755bd7db 100755
--- a/devel/nmbug/nmbug
+++ b/devel/nmbug/nmbug
@@ -169,8 +169,9 @@ class _SubprocessContextManager(object):
                 stream.close()
                 setattr(self._process, name, None)
         status = self._process.wait()
-        _LOG.debug('collect {args} with status {status}'.format(
-            args=self._args, status=status))
+        _LOG.debug(
+            'collect {args} with status {status} (expected {expect})'.format(
+                args=self._args, status=status, expect=self._expect))
         if status not in self._expect:
             raise SubprocessError(args=self._args, status=status)
 
@@ -211,13 +212,14 @@ def _spawn(args, input=None, additional_env=None, wait=False, stdin=None,
             input = input.encode(encoding)
         (stdout, stderr) = p.communicate(input=input)
         status = p.wait()
-        _LOG.debug('collect {args} with status {status}'.format(
-            args=args, status=status))
+        _LOG.debug(
+            'collect {args} with status {status} (expected {expect})'.format(
+                args=args, status=status, expect=expect))
         if stdout is not None:
             stdout = stdout.decode(encoding)
         if stderr is not None:
             stderr = stderr.decode(encoding)
-        if status:
+        if status not in expect:
             raise SubprocessError(
                 args=args, status=status, stdout=stdout, stderr=stderr)
         return (status, stdout, stderr)
-- 
2.13.6

[PATCH 2/4] nmbug: Accept failures to unset core.worktree in clone

[W. Trevor King]

Since 6311cfaf (init: do not set unnecessary core.worktree,
2016-09-25, 2.11.0 [1]), Git has no longer set core.worktree when
--separate-git-dir is used.  This broke clone with:

  $ nmbug clone http://nmbug.notmuchmail.org/git/nmbug-tags.git
  Cloning into '/tmp/nmbug-clone.33gg442e'...
  Checking connectivity: 16674, done.
  ['git', '--git-dir', '/home/wking/.nmbug', 'config', '--unset', 'core.worktree'] exited with 5
  $ echo $?
  1

The initial discussion that lead to the Git change is in [2], and
there is some more discussion around this specific change in [3].
There is some useful background on working trees in this 2009 message
[4].  There is also a git-worktree(1) since df0b6cfb (worktree: new
place for "git prune --worktrees", 2015-06-29, 2.5.0 [5]) which grew
the ability to add new worktrees in 799767cc (Merge branch
'es/worktree-add', 2015-07-13, 2.5.0 [6]).  Folks relying on
core.worktree in the --separate-git-dir case fall into the "former
case" in [4], and as Junio pointed out in that message, Git
operations like 'add' don't really work there.

In nmbug we don't want core.worktree, because our effective working
tree is the notmuch database.  By accepting failed core.worktree
unsets, clone will work with Gits older and younger than 2.11.0.

[1]: https://github.com/git/git/commit/6311cfaf93716bcc43dd1151cb1763e3f80d8099
[2]: https://public-inbox.org/git/CALqjkKZO_y0DNcRJjooyZ7Eso7yBMGhvZ6fE92oO4Su7JeCeng@mail.gmail.com/
[3]: https://public-inbox.org/git/87h94d8cwi.fsf@kyleam.com/
[4]: https://public-inbox.org/git/7viqbsw2vn.fsf@alter.siamese.dyndns.org/
[5]: https://github.com/git/git/commit/df0b6cfbda88144714541664fb501146d6465a82
[6]: https://github.com/git/git/commit/799767cc98b2f8e6f82d0de4bef9b5e8fcc16e97

Reported-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 devel/nmbug/nmbug | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/devel/nmbug/nmbug b/devel/nmbug/nmbug
index 755bd7db..c0e7c3c6 100755
--- a/devel/nmbug/nmbug
+++ b/devel/nmbug/nmbug
@@ -308,7 +308,7 @@ def clone(repository):
                 'git', 'clone', '--no-checkout', '--separate-git-dir', NMBGIT,
                 repository, workdir],
             wait=True)
-    _git(args=['config', '--unset', 'core.worktree'], wait=True)
+    _git(args=['config', '--unset', 'core.worktree'], wait=True, expect=(0, 5))
     _git(args=['config', 'core.bare', 'true'], wait=True)
     _git(args=['branch', 'config', 'origin/config'], wait=True)
 
-- 
2.13.6

[PATCH 3/4] nmbug: Auto-checkout in clone if it wouldn't clobber

[W. Trevor King]

We currently auto-checkout after pull and merge to make those more
convenient.  They're guarded against data-loss with a leading
_insist_committed().  This commit adds the same convenience to clone,
since in most cases users will have no NMBPREFIX-prefixed tags in
their database when they clone.  Users that *do* have
NMBPREFIX-prefixed tags will get a warning (and I've bumped the
default log level to warning so folks who don't set --log-level will
see it) like:

  $ nmbug clone http://nmbug.notmuchmail.org/git/nmbug-tags.git
  Cloning into '/tmp/nmbug-clone.g9dvd0tv'...
  Checking connectivity: 16674, done.
  Branch config set up to track remote branch config from origin.
  Not checking out to avoid clobbering existing tags: notmuch::0.25, ...
---
 devel/nmbug/nmbug | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/devel/nmbug/nmbug b/devel/nmbug/nmbug
index c0e7c3c6..0cd91148 100755
--- a/devel/nmbug/nmbug
+++ b/devel/nmbug/nmbug
@@ -54,7 +54,7 @@ except ImportError:  # Python 2
 __version__ = '0.2'
 
 _LOG = _logging.getLogger('nmbug')
-_LOG.setLevel(_logging.ERROR)
+_LOG.setLevel(_logging.WARNING)
 _LOG.addHandler(_logging.StreamHandler())
 
 NMBGIT = _os.path.expanduser(
@@ -311,6 +311,13 @@ def clone(repository):
     _git(args=['config', '--unset', 'core.worktree'], wait=True, expect=(0, 5))
     _git(args=['config', 'core.bare', 'true'], wait=True)
     _git(args=['branch', 'config', 'origin/config'], wait=True)
+    existing_tags = get_tags()
+    if existing_tags:
+        _LOG.warning(
+            'Not checking out to avoid clobbering existing tags: {}'.format(
+            ', '.join(existing_tags)))
+    else:
+        checkout()
 
 
 def _is_committed(status):
-- 
2.13.6

Re: [PATCH 3/4] nmbug: Auto-checkout in clone if it wouldn't clobber

[David Bremner]

"W. Trevor King" <wking@tremily.us> writes:

> We currently auto-checkout after pull and merge to make those more
> convenient.  They're guarded against data-loss with a leading
> _insist_committed().  This commit adds the same convenience to clone,
> since in most cases users will have no NMBPREFIX-prefixed tags in
> their database when they clone.  Users that *do* have
> NMBPREFIX-prefixed tags will get a warning (and I've bumped the
> default log level to warning so folks who don't set --log-level will
> see it) like:

pushed 1-3 to master. Not sure what happened to patch 4/4?

Re: [PATCH 3/4] nmbug: Auto-checkout in clone if it wouldn't clobber

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1233 bytes --]

On Mon, Dec 11, 2017 at 09:10:47AM -0400, David Bremner wrote:
> Not sure what happened to patch 4/4?

Ah sorry.  My local branch has a commit to bump nmbug's internal
version to 0.3 (see my comments in [1]), but I ended up deciding to
punt that until before the next notmuch release (in case other nmbug
changes landed in the meantime) and forgot to update the denominator.

Based on [2], perhaps it is now time?  Or is [3] likely to land before
the freeze?

Cheers,
Trevor

[1]: id:cover.1507675236.git.wking@tremily.us
     Subject: [PATCH 0/3] nmbug:
     Date: Tue, 10 Oct 2017 15:49:48 -0700
[2]: id:87efo222nr.fsf@tethera.net
     Subject: Notmuch 0.26 release schedule
     Date: Sun, 10 Dec 2017 08:22:32 -0400
[3]: id:4487e001b350aa8e343a1201d869cceca2a03ab6.1508176853.git.wking@tremily.us
     Subject: [PATCH] nmbug: Only error for invalid diff lines in tags/
     Date: Mon, 16 Oct 2017 11:01:47 -0700                                                                                                                                         

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/4] nmbug: Auto-checkout in clone if it wouldn't clobber

[David Bremner]

"W. Trevor King" <wking@tremily.us> writes:

> On Mon, Dec 11, 2017 at 09:10:47AM -0400, David Bremner wrote:
>> Not sure what happened to patch 4/4?
>
> Ah sorry.  My local branch has a commit to bump nmbug's internal
> version to 0.3 (see my comments in [1]), but I ended up deciding to
> punt that until before the next notmuch release (in case other nmbug
> changes landed in the meantime) and forgot to update the denominator.
>
> Based on [2], perhaps it is now time?  Or is [3] likely to land before
> the freeze?
>

I think bumping the version is something reasonable to do during the
feature freeze.

d

Re: [PATCH 0/3] nmbug:

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 255 bytes --]

On Tue 2017-10-10 15:49:48 -0700, W. Trevor King wrote:
> Two changes and a bugfix spun off from today's IRC disussion.

This series looks reasonable to me, from what little i understand of
nmbug.  Thanks for proposing the changes, Trevor.

        --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 0/3] nmbug:

[Jani Nikula]

On Tue, 10 Oct 2017, "W. Trevor King" <wking@tremily.us> wrote:
> I expect the best time to make that bump is just before we cut our
> next notmuch release.  Once we do, we can update the wiki page [1] to
> suggest nmbug 0.3+ and remove the checkout step from “Getting
> started”.

Could we add a "nmbug required version" metadata to the repository, and
add a version check in nmbug? Obviously this doesn't help with 0.2, but,
if we added this at 0.3, we could require a new nmbug version on the
clients.

BR,
Jani.

[PATCH 0/3] nmbug: clone core.worktree and auto-checkout (was: [PATCH 0/3] nmbug:)

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 964 bytes --]

On Sat, Oct 14, 2017 at 03:51:08PM +0300, Jani Nikula wrote:
> Could we add a "nmbug required version" metadata to the repository…

I'm not sure what you mean.  The wiki page will mention the minimum
nmbug version with which the wiki instructions are compatible (as it
already does, [1]).  But that's just for folks who are reading the
wiki and executing the suggested commands.  There's no nmbug
requirement for the repository itself, and the file format inside
nmbug repositories hasn't changed since nmbug was created in ebd1adc5
(contrib/nmbug: new script for sharing tags with a given prefix,
2011-11-12).  If you think we need a clearer version requirement, can
you complete “You need at least this version of nmbug to …”?

Cheers,
Trevor

[1]: https://notmuchmail.org/nmbug/

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]