unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 0/3] nmbug: 
@ 2017-10-10 22:49 W. Trevor King
  2017-10-10 22:49 ` [PATCH 1/4] nmbug: Respect 'expect' in _spawn(..., wait=True) W. Trevor King
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: W. Trevor King @ 2017-10-10 22:49 UTC (permalink / raw)
  To: notmuch; +Cc: Daniel Kahn Gillmor, David Bremner, 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] nmbug: Respect 'expect' in _spawn(..., wait=True)
  2017-10-10 22:49 [PATCH 0/3] nmbug: W. Trevor King
@ 2017-10-10 22:49 ` W. Trevor King
  2017-10-10 22:49 ` [PATCH 2/4] nmbug: Accept failures to unset core.worktree in clone W. Trevor King
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: W. Trevor King @ 2017-10-10 22:49 UTC (permalink / raw)
  To: notmuch; +Cc: Daniel Kahn Gillmor, David Bremner, 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] nmbug: Accept failures to unset core.worktree in clone
  2017-10-10 22:49 [PATCH 0/3] nmbug: W. Trevor King
  2017-10-10 22:49 ` [PATCH 1/4] nmbug: Respect 'expect' in _spawn(..., wait=True) W. Trevor King
@ 2017-10-10 22:49 ` W. Trevor King
  2017-10-10 22:49 ` [PATCH 3/4] nmbug: Auto-checkout in clone if it wouldn't clobber W. Trevor King
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: W. Trevor King @ 2017-10-10 22:49 UTC (permalink / raw)
  To: notmuch; +Cc: Daniel Kahn Gillmor, David Bremner, 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] nmbug: Auto-checkout in clone if it wouldn't clobber
  2017-10-10 22:49 [PATCH 0/3] nmbug: W. Trevor King
  2017-10-10 22:49 ` [PATCH 1/4] nmbug: Respect 'expect' in _spawn(..., wait=True) W. Trevor King
  2017-10-10 22:49 ` [PATCH 2/4] nmbug: Accept failures to unset core.worktree in clone W. Trevor King
@ 2017-10-10 22:49 ` W. Trevor King
  2017-12-11 13:10   ` David Bremner
  2017-10-10 23:15 ` [PATCH 0/3] nmbug: Daniel Kahn Gillmor
  2017-10-14 12:51 ` Jani Nikula
  4 siblings, 1 reply; 10+ messages in thread
From: W. Trevor King @ 2017-10-10 22:49 UTC (permalink / raw)
  To: notmuch; +Cc: Daniel Kahn Gillmor, David Bremner, 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] nmbug:
  2017-10-10 22:49 [PATCH 0/3] nmbug: W. Trevor King
                   ` (2 preceding siblings ...)
  2017-10-10 22:49 ` [PATCH 3/4] nmbug: Auto-checkout in clone if it wouldn't clobber W. Trevor King
@ 2017-10-10 23:15 ` Daniel Kahn Gillmor
  2017-10-14 12:51 ` Jani Nikula
  4 siblings, 0 replies; 10+ messages in thread
From: Daniel Kahn Gillmor @ 2017-10-10 23:15 UTC (permalink / raw)
  To: W. Trevor King, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] nmbug:
  2017-10-10 22:49 [PATCH 0/3] nmbug: W. Trevor King
                   ` (3 preceding siblings ...)
  2017-10-10 23:15 ` [PATCH 0/3] nmbug: Daniel Kahn Gillmor
@ 2017-10-14 12:51 ` Jani Nikula
  2017-10-16 17:05   ` [PATCH 0/3] nmbug: clone core.worktree and auto-checkout (was: [PATCH 0/3] nmbug:) W. Trevor King
  4 siblings, 1 reply; 10+ messages in thread
From: Jani Nikula @ 2017-10-14 12:51 UTC (permalink / raw)
  To: W. Trevor King, notmuch; +Cc: Daniel Kahn Gillmor

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 0/3] nmbug: clone core.worktree and auto-checkout (was: [PATCH 0/3] nmbug:)
  2017-10-14 12:51 ` Jani Nikula
@ 2017-10-16 17:05   ` W. Trevor King
  0 siblings, 0 replies; 10+ messages in thread
From: W. Trevor King @ 2017-10-16 17:05 UTC (permalink / raw)
  To: Jani Nikula; +Cc: notmuch, Daniel Kahn Gillmor

[-- 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 --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] nmbug: Auto-checkout in clone if it wouldn't clobber
  2017-10-10 22:49 ` [PATCH 3/4] nmbug: Auto-checkout in clone if it wouldn't clobber W. Trevor King
@ 2017-12-11 13:10   ` David Bremner
  2017-12-11 17:47     ` W. Trevor King
  0 siblings, 1 reply; 10+ messages in thread
From: David Bremner @ 2017-12-11 13:10 UTC (permalink / raw)
  To: W. Trevor King, notmuch; +Cc: Daniel Kahn Gillmor, W. Trevor King

"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?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] nmbug: Auto-checkout in clone if it wouldn't clobber
  2017-12-11 13:10   ` David Bremner
@ 2017-12-11 17:47     ` W. Trevor King
  2017-12-11 20:51       ` David Bremner
  0 siblings, 1 reply; 10+ messages in thread
From: W. Trevor King @ 2017-12-11 17:47 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch, Daniel Kahn Gillmor

[-- 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 --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] nmbug: Auto-checkout in clone if it wouldn't clobber
  2017-12-11 17:47     ` W. Trevor King
@ 2017-12-11 20:51       ` David Bremner
  0 siblings, 0 replies; 10+ messages in thread
From: David Bremner @ 2017-12-11 20:51 UTC (permalink / raw)
  To: W. Trevor King; +Cc: notmuch, Daniel Kahn Gillmor

"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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-12-11 20:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-10 22:49 [PATCH 0/3] nmbug: W. Trevor King
2017-10-10 22:49 ` [PATCH 1/4] nmbug: Respect 'expect' in _spawn(..., wait=True) W. Trevor King
2017-10-10 22:49 ` [PATCH 2/4] nmbug: Accept failures to unset core.worktree in clone W. Trevor King
2017-10-10 22:49 ` [PATCH 3/4] nmbug: Auto-checkout in clone if it wouldn't clobber W. Trevor King
2017-12-11 13:10   ` David Bremner
2017-12-11 17:47     ` W. Trevor King
2017-12-11 20:51       ` David Bremner
2017-10-10 23:15 ` [PATCH 0/3] nmbug: Daniel Kahn Gillmor
2017-10-14 12:51 ` Jani Nikula
2017-10-16 17:05   ` [PATCH 0/3] nmbug: clone core.worktree and auto-checkout (was: [PATCH 0/3] nmbug:) W. Trevor King

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).