[PATCH] pretty: add format specifiers: %gr, %gt, %gI, gi

2016-07-09 Thread Theodore Ts'o
Add new format specifiers which allow the printing of reflog
timestamp.  This allows us to know when operations which change HEAD
take place (e.g., guilt pop -a, which does the equivalent of a "git
reset --hard commit"), since using %cr will display when the commit
was originally made, instead of when HEAD was moved to that commit.

This allows something like:

git log -g --pretty=format:'%Cred%h%Creset %gd %gs %Cgreen(%gr)%Creset %s' 
--abbrev-commit

to provide what (for me) is a much more useful "git reflog" type of
report.

Signed-off-by: Theodore Ts'o 
---
 Documentation/pretty-formats.txt |  4 
 cache.h  |  1 +
 date.c   |  2 +-
 pretty.c | 18 
 reflog-walk.c| 45 ++--
 reflog-walk.h|  3 +++
 6 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 29b19b9..7927754 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -156,6 +156,10 @@ endif::git-rev-list[]
 - '%gE': reflog identity email (respecting .mailmap, see
   linkgit:git-shortlog[1] or linkgit:git-blame[1])
 - '%gs': reflog subject
+- '%gr': reflog date, relative
+- '%gt': reflog date, UNIX timestamp
+- '%gi': reflog date, ISO 8601-like format
+- '%gI': reflog date, strict ISO 8601 format
 - '%Cred': switch color to red
 - '%Cgreen': switch color to green
 - '%Cblue': switch color to blue
diff --git a/cache.h b/cache.h
index f1dc289..5dd2805 100644
--- a/cache.h
+++ b/cache.h
@@ -1237,6 +1237,7 @@ struct date_mode {
 #define DATE_MODE(t) date_mode_from_type(DATE_##t)
 struct date_mode *date_mode_from_type(enum date_mode_type type);
 
+time_t gm_time_t(unsigned long time, int tz);
 const char *show_date(unsigned long time, int timezone, const struct date_mode 
*mode);
 void show_date_relative(unsigned long time, int tz, const struct timeval *now,
struct strbuf *timebuf);
diff --git a/date.c b/date.c
index 4c7aa9b..f98502e 100644
--- a/date.c
+++ b/date.c
@@ -39,7 +39,7 @@ static const char *weekday_names[] = {
"Sundays", "Mondays", "Tuesdays", "Wednesdays", "Thursdays", "Fridays", 
"Saturdays"
 };
 
-static time_t gm_time_t(unsigned long time, int tz)
+time_t gm_time_t(unsigned long time, int tz)
 {
int minutes;
 
diff --git a/pretty.c b/pretty.c
index 330a5e0..eb1f44e 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1212,6 +1212,24 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
placeholder[1],
c->pretty_ctx->reflog_info,
>pretty_ctx->date_mode);
+   case 'r':   /* date, relative */
+   strbuf_addstr(sb,
+   show_reflog_date(c->pretty_ctx->reflog_info,
+   DATE_MODE(RELATIVE)));
+   return 2;
+   case 'i':   /* date, ISO 8601-like */
+   strbuf_addstr(sb,
+   show_reflog_date(c->pretty_ctx->reflog_info,
+   DATE_MODE(ISO8601)));
+   return 2;
+   case 'I':   /* date, ISO 8601 strict */
+   strbuf_addstr(sb,
+   show_reflog_date(c->pretty_ctx->reflog_info,
+   DATE_MODE(ISO8601_STRICT)));
+   return 2;
+   case 't':
+   strbuf_addf(sb, "%lu", 
get_reflog_time_t(c->pretty_ctx->reflog_info));
+   return 2;
}
return 0;   /* unknown %g placeholder */
case 'N':
diff --git a/reflog-walk.c b/reflog-walk.c
index a246af2..d0aa2d0 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -292,17 +292,24 @@ void get_reflog_selector(struct strbuf *sb,
strbuf_addch(sb, '}');
 }
 
-void get_reflog_message(struct strbuf *sb,
-   struct reflog_walk_info *reflog_info)
+static struct reflog_info *get_reflog_info(struct reflog_walk_info 
*reflog_info)
 {
struct commit_reflog *commit_reflog = reflog_info->last_commit_reflog;
-   struct reflog_info *info;
-   size_t len;
 
if (!commit_reflog)
-   return;
+   return NULL;
+
+   return _reflog->reflogs->items[commit_reflog->recno+1];
+}
 
-   info = _reflog->reflogs->items[commit_reflog->recno+1];
+void get_reflog_message(struct strbuf *sb,
+   struct reflog_walk_info *reflog_info)
+{
+   struct reflog_info *info = get_reflog_info(reflog_info);
+   size_t len;
+
+   if (!info)
+   return NULL;
len = strlen(info->message);
if (len > 0)
  

Re: What's cooking in git.git (Jul 2016, #03; Fri, 8)

2016-07-09 Thread Jeff King
On Sun, Jul 10, 2016 at 03:47:36AM +, Eric Wong wrote:

> > There was one minor improvement I suggested[1] (and which you seemed to
> > like), which is to push the errno check into the function. That wasn't
> > expressed in patch form, though, so I've included below a version of
> > your patch with my suggestion squashed in.
> 
> Yes, I'm fine with either, but I'm slightly thrown off by
> a function relying on errno being set by the caller, even if it
> is errno.  So maybe localizing it is better (see below)

Yeah, I had a similar reservation, but didn't want to clutter the
interface. However, just passing errno isn't too bad (as you showed
below), and is much less magical.

Do you want to squash that and re-send the whole patch to make Junio's
life easier?

> > Since both conditionals just call "continue", you could actually fold
> > them into a single if() in each caller, but I think it's easier to
> > follow as two separate ones.
> > 
> > You could actually fold the t
> 
> Copy-paste error?

Nope, just typing error while revising. :)

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-gui: Do not reset author details on amend

2016-07-09 Thread Orgad Shaneh
It's been over 2 months. Can anyone please review and merge it?

Thanks.
- Orgad

On Wed, May 18, 2016 at 9:12 AM, Orgad Shaneh  wrote:
> ping?
>
> On Thu, May 5, 2016 at 8:22 PM, Junio C Hamano  wrote:
>> Pat, we haven't heard from you for a long time.  Are you still
>> around and interested in helping us by maintaining git-gui?
>>
>> Otherwise we may have to start recruiting a volunteer or two to take
>> this over.
>>
>> Thanks.
>>
>> Orgad Shaneh  writes:
>>
>>> git commit --amend preserves the author details unless --reset-author is
>>> given.
>>>
>>> git-gui discards the author details on amend.
>>>
>>> Fix by reading the author details along with the commit message, and
>>> setting the appropriate environment variables required for preserving
>>> them.
>>>
>>> Reported long ago in the mailing list[1].
>>>
>>> [1] http://article.gmane.org/gmane.comp.version-control.git/243921
>>>
>>> Signed-off-by: Orgad Shaneh 
>>> ---
>>>  git-gui/lib/commit.tcl | 19 +++
>>>  1 file changed, 19 insertions(+)
>>>
>>> diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl
>>> index 864b687..60edf99 100644
>>> --- a/git-gui/lib/commit.tcl
>>> +++ b/git-gui/lib/commit.tcl
>>> @@ -1,8 +1,13 @@
>>>  # git-gui misc. commit reading/writing support
>>>  # Copyright (C) 2006, 2007 Shawn Pearce
>>>
>>> +set author_name ""
>>> +set author_email ""
>>> +set author_date ""
>>> +
>>>  proc load_last_commit {} {
>>>   global HEAD PARENT MERGE_HEAD commit_type ui_comm
>>> + global author_name author_email author_date
>>>   global repo_config
>>>
>>>   if {[llength $PARENT] == 0} {
>>> @@ -34,6 +39,10 @@ You are currently in the middle of a merge that has not 
>>> been fully completed.  Y
>>>   lappend parents [string range $line 7 
>>> end]
>>>   } elseif {[string match {encoding *} $line]} {
>>>   set enc [string tolower [string range 
>>> $line 9 end]]
>>> + } elseif {[regexp "author 
>>> (.*)\\s<(.*)>\\s(\\d.*$)" $line all name email time]} {
>>> + set author_name $name
>>> + set author_email $email
>>> + set author_date $time
>>>   }
>>>   }
>>>   set msg [read $fd]
>>> @@ -107,8 +116,12 @@ proc do_signoff {} {
>>>
>>>  proc create_new_commit {} {
>>>   global commit_type ui_comm
>>> + global author_name author_email author_date
>>>
>>>   set commit_type normal
>>> + set author_name ""
>>> + set author_email ""
>>> + set author_date ""
>>>   $ui_comm delete 0.0 end
>>>   $ui_comm edit reset
>>>   $ui_comm edit modified false
>>> @@ -327,6 +340,7 @@ proc commit_committree {fd_wt curHEAD msg_p} {
>>>   global ui_comm selected_commit_type
>>>   global file_states selected_paths rescan_active
>>>   global repo_config
>>> + global env author_name author_email author_date
>>>
>>>   gets $fd_wt tree_id
>>>   if {[catch {close $fd_wt} err]} {
>>> @@ -366,6 +380,11 @@ A rescan will be automatically started now.
>>>   }
>>>   }
>>>
>>> + if {$author_name ne ""} {
>>> + set env(GIT_AUTHOR_NAME) $author_name
>>> + set env(GIT_AUTHOR_EMAIL) $author_email
>>> + set env(GIT_AUTHOR_DATE) $author_date
>>> + }
>>>   # -- Create the commit.
>>>   #
>>>   set cmd [list commit-tree $tree_id]
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [ANNOUNCE] more archives of this list

2016-07-09 Thread Eric Wong
Eric Wong  wrote:
>   https://public-inbox.org/.temp/git.vger.kernel.org-6c38c917e55c.gz
>   (362M)
> 
>   git init --bare mirror.git
>   curl $FAST_EXPORT_GZ_URL | git --git-dir=mirror.git fast-import

Oops, that is missing zcat:

curl $FAST_EXPORT_GZ_URL | zcat | git --git-dir=mirror.git fast-import

>   git --git-dir=mirror.git remote add --mirror=fetch origin $URL

And I forgot to set a branch for fast-export and just exported
a ref, so importers will need to create master explicitly:

git update-ref refs/heads/master 6c38c917e55c
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jul 2016, #03; Fri, 8)

2016-07-09 Thread Eric Wong
Jeff King  wrote:
> On Sat, Jul 09, 2016 at 11:45:18PM +, Eric Wong wrote:
> > Junio C Hamano  wrote:
> > > * sb/submodule-parallel-fetch (2016-06-27) 2 commits
> > >   (merged to 'next' on 2016-07-06 at de5fd35)
> > >  + xwrite: poll on non-blocking FDs
> > >  + xread: retry after poll on EAGAIN/EWOULDBLOCK

> > Any thoughts on my cleanup 3/2 patch for this series?
> > ("hoist out io_wait function for xread and xwrite")
> > https://public-inbox.org/git/20160627201311.ga7...@dcvr.yhbt.net/raw
> > 
> > Jeff liked it:
> > https://public-inbox.org/git/20160627214947.ga17...@sigill.intra.peff.net/
> 
> I did (and do) like it. I think it may have simply gotten lost in the
> mass of "should we handle EAGAIN from getdelim()" patches I sent (to
> which I concluded "probably not").
> 
> There was one minor improvement I suggested[1] (and which you seemed to
> like), which is to push the errno check into the function. That wasn't
> expressed in patch form, though, so I've included below a version of
> your patch with my suggestion squashed in.

Yes, I'm fine with either, but I'm slightly thrown off by
a function relying on errno being set by the caller, even if it
is errno.  So maybe localizing it is better (see below)

> I didn't include the test I mentioned in [2]. I think the potential for
> portability and raciness problems make it probably not worth the
> trouble.

Agreed.

> ---
> Since both conditionals just call "continue", you could actually fold
> them into a single if() in each caller, but I think it's easier to
> follow as two separate ones.
> 
> You could actually fold the t

Copy-paste error?

> +static int handle_nonblock(int fd, short poll_events)
> +{
> + struct pollfd pfd;
> +
> + if (errno != EAGAIN && errno != EWOULDBLOCK)
> + return 0;
> +

I prefer localizing errno and having the caller pass it
explicitly:

static int handle_nonblock(int fd, short poll_events, int err)
{
struct pollfd pfd;

if (err != EAGAIN && err != EWOULDBLOCK)
return 0;

And the callers would pass errno:

if (handle_nonblock(fd, POLLIN, errno))
continue;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jul 2016, #03; Fri, 8)

2016-07-09 Thread Jeff King
On Sat, Jul 09, 2016 at 11:45:18PM +, Eric Wong wrote:

> Junio C Hamano  wrote:
> > * sb/submodule-parallel-fetch (2016-06-27) 2 commits
> >   (merged to 'next' on 2016-07-06 at de5fd35)
> >  + xwrite: poll on non-blocking FDs
> >  + xread: retry after poll on EAGAIN/EWOULDBLOCK
> > 
> >  Fix a recently introduced codepaths that are involved in parallel
> >  submodule operations, which gave up on reading too early, and
> >  could have wasted CPU while attempting to write under a corner case
> >  condition.
> > 
> >  Will merge to 'master'.
> 
> Any thoughts on my cleanup 3/2 patch for this series?
> ("hoist out io_wait function for xread and xwrite")
> https://public-inbox.org/git/20160627201311.ga7...@dcvr.yhbt.net/raw
> 
> Jeff liked it:
> https://public-inbox.org/git/20160627214947.ga17...@sigill.intra.peff.net/

I did (and do) like it. I think it may have simply gotten lost in the
mass of "should we handle EAGAIN from getdelim()" patches I sent (to
which I concluded "probably not").

There was one minor improvement I suggested[1] (and which you seemed to
like), which is to push the errno check into the function. That wasn't
expressed in patch form, though, so I've included below a version of
your patch with my suggestion squashed in.

I didn't include the test I mentioned in [2]. I think the potential for
portability and raciness problems make it probably not worth the
trouble.

[1] https://public-inbox.org/git/2016062738.ga23...@sigill.intra.peff.net

[2] https://public-inbox.org/git/20160627214947.ga17...@sigill.intra.peff.net

-- >8 --
From: Eric Wong 
Subject: [PATCH] hoist out io_wait function for xread and xwrite

At least for me, this improves the readability of xread and
xwrite; hopefully allowing missing "continue" statements to
be spotted more easily.

Signed-off-by: Eric Wong 
Signed-off-by: Jeff King 
---
Since both conditionals just call "continue", you could actually fold
them into a single if() in each caller, but I think it's easier to
follow as two separate ones.

You could actually fold the t
 wrapper.c | 48 
 1 file changed, 20 insertions(+), 28 deletions(-)

diff --git a/wrapper.c b/wrapper.c
index f7ea6c4..0a966d6 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -224,6 +224,24 @@ int xopen(const char *path, int oflag, ...)
}
 }
 
+static int handle_nonblock(int fd, short poll_events)
+{
+   struct pollfd pfd;
+
+   if (errno != EAGAIN && errno != EWOULDBLOCK)
+   return 0;
+
+   pfd.fd = fd;
+   pfd.events = poll_events;
+
+   /*
+* no need to check for errors, here;
+* a subsequent read/write will detect unrecoverable errors
+*/
+   poll(, 1, -1);
+   return 1;
+}
+
 /*
  * xread() is the same a read(), but it automatically restarts read()
  * operations with a recoverable error (EAGAIN and EINTR). xread()
@@ -239,21 +257,8 @@ ssize_t xread(int fd, void *buf, size_t len)
if (nr < 0) {
if (errno == EINTR)
continue;
-   if (errno == EAGAIN || errno == EWOULDBLOCK) {
-   struct pollfd pfd;
-   pfd.events = POLLIN;
-   pfd.fd = fd;
-   /*
-* it is OK if this poll() failed; we
-* want to leave this infinite loop
-* only when read() returns with
-* success, or an expected failure,
-* which would be checked by the next
-* call to read(2).
-*/
-   poll(, 1, -1);
+   if (handle_nonblock(fd, POLLIN))
continue;
-   }
}
return nr;
}
@@ -274,21 +279,8 @@ ssize_t xwrite(int fd, const void *buf, size_t len)
if (nr < 0) {
if (errno == EINTR)
continue;
-   if (errno == EAGAIN || errno == EWOULDBLOCK) {
-   struct pollfd pfd;
-   pfd.events = POLLOUT;
-   pfd.fd = fd;
-   /*
-* it is OK if this poll() failed; we
-* want to leave this infinite loop
-* only when write() returns with
-* success, or an expected failure,
-* which would be checked by the next
-* call to write(2).
-*/
-   poll(, 1, -1);
+

[ANNOUNCE] more archives of this list

2016-07-09 Thread Eric Wong
Very much a work-in-progress, but NNTP and HTTP/HTTPS sorta work
based on stuff that is on gmane and stuff I'm accumulating by
being a subscriber.

The first two Tor hidden service onions are actually on better
hardware than the non-hidden public-inbox.org one:

nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
nntp://news.public-inbox.org/inbox.comp.version-control.git

http://czquwvybam4bgbro.onion/git
http://hjrcffqmbrq6wope.onion/git
http://ou63pmih66umazou.onion/git
https://public-inbox.org/git/

HTTP URLs are clonable, but I've generated the following fast-export dump:

https://public-inbox.org/.temp/git.vger.kernel.org-6c38c917e55c.gz
(362M)

git init --bare mirror.git
curl $FAST_EXPORT_GZ_URL | git --git-dir=mirror.git fast-import
git --git-dir=mirror.git remote add --mirror=fetch origin $URL

I recommend bare repos for importing, since the trees consist of
2/38 SHA-1 paths of Message-IDs and there's nearly 300K messages.

In contrast, bundles and packs delta poorly and only get down
around 750-800M with aggressive packing
(And I haven't done that in a while.)


Code is AGPL-3.0+: git clone https://public-inbox.org/


Additional mirrors or forks (perhaps different UIs) are very welcome,
as I expect none of my servers or network connections to be reliable.


I have the "public-inbox-watch" command running in screen
watching my Maildirs, it uses a config file which is parseable/writable
using git-config:

==> ~/.public-inbox/config <==
[publicinboxlearn]
; spam gets moved here for auto-removal:
watchspam = maildir:/path/to/maildirs/.INBOX.spam
[publicinboxwatch]
; optional, adds some additional spam checking
spamcheck = spamc
[publicinbox "git"]
; git repo for this list
mainrepo = /path/to/mirror.git

; this removes the list footer signature:
filter = PublicInbox::Filter::Vger

; this is where my git-related mail goes (some of it is from Debian)
watch = maildir:/path/to/maildirs/.INBOX.git

; only match messages with the correct List-Id header:
watchheader = List-Id:

; next 4 lines are only necessary for HTTP and NNTP servers
address = git@vger.kernel.org
url = http://ou63pmih66umazou.onion/git
newsgroup = inbox.comp.version-control.git
infourl = http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jul 2016, #03; Fri, 8)

2016-07-09 Thread Eric Wong
Junio C Hamano  wrote:
> * sb/submodule-parallel-fetch (2016-06-27) 2 commits
>   (merged to 'next' on 2016-07-06 at de5fd35)
>  + xwrite: poll on non-blocking FDs
>  + xread: retry after poll on EAGAIN/EWOULDBLOCK
> 
>  Fix a recently introduced codepaths that are involved in parallel
>  submodule operations, which gave up on reading too early, and
>  could have wasted CPU while attempting to write under a corner case
>  condition.
> 
>  Will merge to 'master'.

Any thoughts on my cleanup 3/2 patch for this series?
("hoist out io_wait function for xread and xwrite")
https://public-inbox.org/git/20160627201311.ga7...@dcvr.yhbt.net/raw

Jeff liked it:
https://public-inbox.org/git/20160627214947.ga17...@sigill.intra.peff.net/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Over-/underquoting, was Re: [PATCH] revert: clarify seemingly bogus OPT_END repetition

2016-07-09 Thread Eric Wong
Michael J Gruber  wrote:
> 2016-07-06 22:15 GMT+02:00 Jacob Keller :
> > On Wed, Jul 6, 2016 at 6:16 AM, Johannes Schindelin
> >  wrote:
> ...
> >>> It is not unheard of that a MUA can collapse and expand properly quoted
> >>> parts on request...
> >>
> >> Sure. Show me some kick-ass, scriptable mail client and I will have a
> >> look.

I like mutt since it lets me pipe stuff to arbitrary commands,
"T" works in the pager for toggling quoted text.

> > Yep, I haven't found a mail client that really works for me either :(
> 
> Every other day I want to get rid of Thunderbird... always on the
> verge of going (back) to mutt, though I do need some calendar
> integration and such.

It's possible to use both with IMAP (offlineimap + local dovecot works
well on slow connections).

I rely on at+atd to email myself and also maintain a text file over IMAP.

> Until the day when the scriptable mua we wish for comes upon us, may I
> recommend the "QuoteCollapse" extension to Thunderbird that transforms
> those huge quotes into neat short headlines with a twist(y).

The "T" (toggle-quoted) command in mutt hides quoted entirely, which
makes it a bit confusing, at times; but toggling again unhides it.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jul 2016, #03; Fri, 8)

2016-07-09 Thread Eric Wong
Junio C Hamano  wrote:
> * ew/svn-bad-ref (2016-07-06) 1 commit
>  - git-svn: warn instead of dying when commit data is missing

>  I'm likely to discard this, favouring a direct pull request from
>  the subsystem maintainer directly going to 'master'.

Pushed with your sign-off added, along with Christopher's patch.

I was hoping to work on the Apache 2.4 test suite fixes Michael
started working on last year this week, maybe next (or very soon)
since most of my machines are finally upgraded to Debian Jessie.

The following changes since commit cf4c2cfe52be5bd973a4838f73a35d3959ce2f43:

  Second batch of topics for 2.10 (2016-06-27 10:07:08 -0700)

are available in the git repository at:

  git://bogomips.org/git-svn.git master

for you to fetch changes up to 2af7da9f8fb68337030630d88c19db512189babc:

  git-svn: warn instead of dying when commit data is missing (2016-07-09 
22:53:54 +)


Christopher Layne (1):
  git-svn: clone: Fail on missing url argument

Eric Wong (1):
  git-svn: warn instead of dying when commit data is missing

 git-svn.perl| 5 -
 perl/Git/SVN.pm | 8 ++--
 2 files changed, 10 insertions(+), 3 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] guilt: fix portability problem with using find -perm +111

2016-07-09 Thread Theodore Ts'o
GNU find no longers accepts -perm +111, even though the rest of the
world (MacOS, Solaris, BSD) still do.  Workaround this problem by
using -executable if the system find utility will accept it.

Signed-off-by: Theodore Ts'o 
---
 guilt | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/guilt b/guilt
index 38d426b..b90f02d 100755
--- a/guilt
+++ b/guilt
@@ -73,8 +73,17 @@ GUILT_PATH="$(dirname "$0")"
 
 guilt_commands()
 {
-   find "$GUILT_PATH/../lib/guilt" -maxdepth 1 -name "guilt-*" -type f 
-perm +111 2> /dev/null | sed -e "s/.*\\/$GUILT-//"
-   find "$GUILT_PATH" -maxdepth 1 -name "guilt-*" -type f -perm +111 | sed 
-e "s/.*\\/$GUILT-//"
+   # GNU Find no longer accepts -perm +111, even though the rest
+   # world (MacOS, Solaris, BSD, etc.) does.  Sigh.  Using -executable
+   # is arugably better, but it is a GNU extension.  Since this isn't
+   # a fast path and guilt doesn't use autoconf, test for it as needed.
+   if find . -maxdepth 0 -executable > /dev/null 2>&1 ; then
+   exe_test="-executable"
+   else
+   exe_test="-find +111"
+   fi
+   find "$GUILT_PATH/../lib/guilt" -maxdepth 1 -name "guilt-*" -type f 
$exe_test 2> /dev/null | sed -e "s/.*\\/$GUILT-//"
+   find "$GUILT_PATH" -maxdepth 1 -name "guilt-*" -type f $exe_test | sed 
-e "s/.*\\/$GUILT-//"
 }
 
 # by default, we shouldn't fail
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] guilt: update reflog with annotations of guilt-command being run

2016-07-09 Thread Theodore Ts'o
Many of the updates made by guilt use git update-ref, which means that
the output of "git reflog" is extremely unedifying, e.g:

ff0031d HEAD@{177}: reset: moving to ff0031d848a0cd7002606f9feef958de8d5edf19
90f4305 HEAD@{178}:
a638d43 HEAD@{179}:
ff0031d HEAD@{180}:
079788d HEAD@{181}:
87a6280 HEAD@{182}:
5b9554d HEAD@{183}:
de9e918 HEAD@{184}: reset: moving to de9e9181bc066d63d78b768e95b5d949e2a8673a
5b9554d HEAD@{185}:

So teach guilt to use the "set_reflog_action" helper, and since
git-update-ref doesn't respect the GIT_REFLOG_ACTION environment
variable, use its -m option so that "git reflog" can look like this
instead:

1eaa566 HEAD@{11}: guilt-push: track-more-dependencies-on-transaction-commit
ab714af HEAD@{12}: guilt-push: move-lockdep-tracking-to-journal_s
7a4b188 HEAD@{13}: guilt-push: move-lockdep-instrumentation-for-jbd2-handles
78d9625 HEAD@{14}: guilt-push: respect-nobarrier-mount-option-in-nojournal-mode
d08854f HEAD@{15}: guilt-pop: updating HEAD
d08854f HEAD@{16}: guilt-pop: updating HEAD
d08854f HEAD@{17}: guilt-push: 
optimize-ext4_should_retry_alloc-to-improve-ENOSPC-performance

Signed-off-by: Theodore Ts'o 
Cc: Josef 'Jeff' Sipek 
---
 guilt | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/guilt b/guilt
index 35177b9..38d426b 100755
--- a/guilt
+++ b/guilt
@@ -114,6 +114,7 @@ if [ $# -ne 0 ]; then
disp "" >&2
exit 1
fi
+   set_reflog_action "guilt-$CMDNAME"
 
shift
 else
@@ -640,7 +641,7 @@ commit()
commitish=`git commit-tree $treeish -p $2 < "$TMP_MSG"`
if $old_style_prefix || git rev-parse --verify --quiet 
refs/heads/$GUILT_PREFIX$branch >/dev/null
then
-   git update-ref HEAD $commitish
+   git update-ref -m "$GIT_REFLOG_ACTION" HEAD $commitish
else
git branch $GUILT_PREFIX$branch $commitish
git symbolic-ref HEAD refs/heads/$GUILT_PREFIX$branch
@@ -687,7 +688,8 @@ push_patch()
fi
fi
 
-   commit "$pname" HEAD
+   GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: $pname" \
+   commit "$pname" HEAD
 
echo "$pname" >> "$applied"
 
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: gc and repack ignore .git/*HEAD when checking reachability

2016-07-09 Thread Duy Nguyen
On Sat, Jul 9, 2016 at 4:09 PM, Josh Triplett  wrote:
> On Sat, Jul 09, 2016 at 09:35:24AM +0200, Johannes Schindelin wrote:
>> On Fri, 8 Jul 2016, Junio C Hamano wrote:
>> > Josh Triplett  writes:
>> >
>> > > That sounds reasonable.  And if they *do* end up taking any time to
>> > > traverse, it's because they weren't reachable from other anchoring
>> > > points, so taking the extra time to traverse them seems fine.
>> >
>> > The only thing that is hard is to clearly define _what_ are the new
>> > anchoring points.
>> >
>> > It cannot be "anything directly under .git that has all-caps name
>> > that ends with _HEAD".  The ones we write we know are going to be
>> > removed at some point in time (e.g. "git reset", "git bisect reset",
>> > "git merge --abort", etc.).  We do not have any control on random
>> > ones that the users and third-party tools leave behind, holding onto
>> > irrelevant objects forever.
>>
>> Please note that bisect already uses the (transient) refs/bisect/
>> namespace. So I do not think we need to take specific care of the
>> BISECT_* files.
>>
>> If we had thought of it back then, we could have used such a transient
>> namespace also for FETCH_HEAD, CHERRY_PICK_HEAD and also for detached
>> HEADs (which we should have called "unnamed branches").
>>
>> Now, how about special-casing *just* these legacy files in gc: HEAD,
>> FETCH_HEAD, MERGE_HEAD and CHERRY_PICK_HEAD? Any new transient refs should
>> live in the refs/ namespace, which is already handled.
>
> That seems workable as well; in that case, we should also document this
> (in the git-gc manpage at a minimum), and explicitly suggest creating
> refs in refs/ but outside of refs/heads/ and refs/tags/, rather than
> directly in .git/.

Not just outside refs/heads and refs/tags. It has to be in a specified
namespace like refs/worktree/ or something (we are close to be ready
for that). We could update the man page about git-gc shortcomings now,
but I think we should wait until refs/worktree (or something like
that) becomes true before suggesting more.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] worktree: use strbuf_add_absolute_path() directly

2016-07-09 Thread René Scharfe
absolute_path() is a wrapper for strbuf_add_absolute_path().  Call the
latter directly for adding absolute paths to a strbuf.  That's shorter
and avoids an extra string copy.

Signed-off-by: Rene Scharfe 
---
 worktree.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/worktree.c b/worktree.c
index e2a94e0..b819baf 100644
--- a/worktree.c
+++ b/worktree.c
@@ -80,7 +80,7 @@ static struct worktree *get_main_worktree(void)
int is_bare = 0;
int is_detached = 0;
 
-   strbuf_addstr(_path, absolute_path(get_git_common_dir()));
+   strbuf_add_absolute_path(_path, get_git_common_dir());
is_bare = !strbuf_strip_suffix(_path, "/.git");
if (is_bare)
strbuf_strip_suffix(_path, "/.");
@@ -125,7 +125,7 @@ static struct worktree *get_linked_worktree(const char *id)
strbuf_rtrim(_path);
if (!strbuf_strip_suffix(_path, "/.git")) {
strbuf_reset(_path);
-   strbuf_addstr(_path, absolute_path("."));
+   strbuf_add_absolute_path(_path, ".");
strbuf_strip_suffix(_path, "/.");
}
 
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Over-/underquoting, was Re: [PATCH] revert: clarify seemingly bogus OPT_END repetition

2016-07-09 Thread Michael J Gruber
2016-07-06 22:15 GMT+02:00 Jacob Keller :
> On Wed, Jul 6, 2016 at 6:16 AM, Johannes Schindelin
>  wrote:
...
>>> It is not unheard of that a MUA can collapse and expand properly quoted
>>> parts on request...
>>
>> Sure. Show me some kick-ass, scriptable mail client and I will have a
>> look.
>>
>
> Yep, I haven't found a mail client that really works for me either :(

Every other day I want to get rid of Thunderbird... always on the
verge of going (back) to mutt, though I do need some calendar
integration and such.

Until the day when the scriptable mua we wish for comes upon us, may I
recommend the "QuoteCollapse" extension to Thunderbird that transforms
those huge quotes into neat short headlines with a twist(y).

Michael
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] rm: reuse strbuf for all remove_dir_recursively() calls

2016-07-09 Thread René Scharfe
Don't throw the memory allocated for remove_dir_recursively() away after
a single call, use it for the other entries as well instead.

Signed-off-by: Rene Scharfe 
---
 builtin/rm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index 8abb020..b2fee3e 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -387,6 +387,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 */
if (!index_only) {
int removed = 0, gitmodules_modified = 0;
+   struct strbuf buf = STRBUF_INIT;
for (i = 0; i < list.nr; i++) {
const char *path = list.entry[i].name;
if (list.entry[i].is_submodule) {
@@ -398,7 +399,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
continue;
}
} else {
-   struct strbuf buf = STRBUF_INIT;
+   strbuf_reset();
strbuf_addstr(, path);
if (!remove_dir_recursively(, 0)) {
removed = 1;
@@ -410,7 +411,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
/* Submodule was removed by 
user */
if 
(!remove_path_from_gitmodules(path))
gitmodules_modified = 1;
-   strbuf_release();
/* Fallthrough and let remove_path() 
fail. */
}
}
@@ -421,6 +421,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
if (!removed)
die_errno("git rm: '%s'", path);
}
+   strbuf_release();
if (gitmodules_modified)
stage_updated_gitmodules();
}
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: reflogs and worktrees?

2016-07-09 Thread Johannes Schindelin
Hi Duy,

On Tue, 5 Jul 2016, Duy Nguyen wrote:

> On Tue, Jul 5, 2016 at 5:07 PM, Johannes Schindelin
>  wrote:
> > Hi Duy,
> >
> > ever since I started working extensively with worktrees, I end up with
> > these funny gc problems, like broken links and stale reflogs.
> 
> Yeah we have problem with gc not traversing all worktree refs.

That is a bit of an understatement.

I get tons of errors like this one, just from the regular auto gc:

error: Could not read 296ee31af712b02469c4bb606fbf2fac229bcca6
fatal: Failed to traverse parents of commit 
3c22e84e2ccdec5d8243344fc4ec68942b87a393
error: failed to run repack

I still have to address all of them, in particular because we do not (yet)
have tools to identify how certain objects are expected to be reachable
when they are missing (see my `gc-worktree` branch for my current progress
of teaching git-fsck to describe the broken links better).

For the moment, I am working around this problem with the following hook
(wrapped into a unit test to verify that it does what I expect it to do):

-- snipsnap --
Subject: [PATCH] Demonstrate a workaround for the worktree/gc problem

When gc --auto is called in the presence of worktrees, pretty much all of
the reflogs go to hell.

In the --auto case, we can install a hook that runs before-hand,
accumulates all the worktree-specific refs and reflogs and installs them
into a very special reflog in the common refs namespace. The only purpose
of this stunt is to let gc pick up on those refs and reflogs, of course,
and *not* ignore them.

Unfortunately, this still does not address the "git gc" case, but
hopefully a real fix will be here some time soon.

Signed-off-by: Johannes Schindelin 
---
 t/t6500-gc.sh | 63 +++
 1 file changed, 63 insertions(+)

diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 5d7d414..518e809 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -43,4 +43,67 @@ test_expect_success 'gc is not aborted due to a stale 
symref' '
)
 '
 
+test_expect_success 'install pre-auto-gc hook for worktrees' '
+   mkdir -p .git/hooks &&
+   write_script .git/hooks/pre-auto-gc <<-\EOF
+   echo "Preserving refs/reflogs of worktrees" >&2 &&
+   dir="$(git rev-parse --git-common-dir)" &&
+   refsdir="$dir/logs/refs" &&
+   rm -f "$refsdir"/preserve &&
+   ident="$(GIT_COMMITTER_DATE= git var GIT_COMMITTER_IDENT)" &&
+   (
+   find "$dir"/logs "$dir"/worktrees/*/logs \
+   -type f -exec cat {} \; |
+   cut -d" " -f1
+   find "$dir"/HEAD "$dir"/worktrees/*/HEAD "$dir"/refs \
+   "$dir"/worktrees/*/refs -type f -exec cat {} \; |
+   grep -v "/^ref:/"
+   ) 2>/dev/null |
+   sort |
+   uniq |
+   sed "s/.*/& & $identdummy/" >"$dir"/preserve &&
+   mkdir -p "$refsdir" &&
+   mv "$dir"/preserve "$refsdir"/
+   EOF
+'
+
+trigger_auto_gc () {
+   # This is unfortunately very, very ugly
+   gdir="$(git rev-parse --git-common-dir)" &&
+   mkdir -p "$gdir"/objects/17 &&
+   touch "$gdir"/objects/17/17171717171717171717171717171717171717 &&
+   touch "$gdir"/objects/17/17171717171717171717171717171717171718 &&
+   git -c gc.auto=1 -c gc.pruneexpire=now -c gc.autodetach=0 gc --auto
+}
+
+test_expect_success 'gc respects refs/reflogs in all worktrees' '
+   test_commit something &&
+   git worktree add worktree &&
+   (
+   cd worktree &&
+   git checkout --detach &&
+   echo 1 >something.t &&
+   test_tick &&
+   git commit -m worktree-reflog something.t &&
+   git rev-parse --verify HEAD >../commit-reflog &&
+   echo 2 >something.t &&
+   test_tick &&
+   git commit -m worktree-ref something.t &&
+   git rev-parse --verify HEAD >../commit-ref
+   ) &&
+   trigger_auto_gc &&
+   git rev-parse --verify $(cat commit-ref)^{commit} &&
+   git rev-parse --verify $(cat commit-reflog)^{commit} &&
+
+   # Now, add a reflog in the top-level dir and verify that `git gc` in
+   # the worktree does not purge that
+   git checkout --detach &&
+   echo 3 >something.t &&
+   test_tick &&
+   git commit -m commondir-reflog something.t &&
+   git rev-parse --verify HEAD >commondir-reflog &&
+   (cd worktree && trigger_auto_gc) &&
+   git rev-parse --verify $(cat commondir-reflog)^{commit}
+'
+
 test_done
-- 
2.9.0.278.g1caae67

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] am: ignore return value of write_file()

2016-07-09 Thread Johannes Schindelin
Hi Peff,

On Fri, 8 Jul 2016, Jeff King wrote:

> I think we can clean that up, though. I'll hopefully have a series in a
> few minutes.

You caught me at busy times... I'll review it tomorrow, promise!

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: gc and repack ignore .git/*HEAD when checking reachability

2016-07-09 Thread Josh Triplett
On Sat, Jul 09, 2016 at 09:35:24AM +0200, Johannes Schindelin wrote:
> On Fri, 8 Jul 2016, Junio C Hamano wrote:
> > Josh Triplett  writes:
> > 
> > > That sounds reasonable.  And if they *do* end up taking any time to
> > > traverse, it's because they weren't reachable from other anchoring
> > > points, so taking the extra time to traverse them seems fine.
> > 
> > The only thing that is hard is to clearly define _what_ are the new
> > anchoring points.
> > 
> > It cannot be "anything directly under .git that has all-caps name
> > that ends with _HEAD".  The ones we write we know are going to be
> > removed at some point in time (e.g. "git reset", "git bisect reset",
> > "git merge --abort", etc.).  We do not have any control on random
> > ones that the users and third-party tools leave behind, holding onto
> > irrelevant objects forever.
> 
> Please note that bisect already uses the (transient) refs/bisect/
> namespace. So I do not think we need to take specific care of the
> BISECT_* files.
> 
> If we had thought of it back then, we could have used such a transient
> namespace also for FETCH_HEAD, CHERRY_PICK_HEAD and also for detached
> HEADs (which we should have called "unnamed branches").
> 
> Now, how about special-casing *just* these legacy files in gc: HEAD,
> FETCH_HEAD, MERGE_HEAD and CHERRY_PICK_HEAD? Any new transient refs should
> live in the refs/ namespace, which is already handled.

That seems workable as well; in that case, we should also document this
(in the git-gc manpage at a minimum), and explicitly suggest creating
refs in refs/ but outside of refs/heads/ and refs/tags/, rather than
directly in .git/.

> BTW this issue is getting much more problematic when you have a lot of
> worktrees, some of which operate on detached HEADs. Which I do.
> 
> Ciao,
> Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] log: decorate HEAD -> branch with the same color for arrow and HEAD

2016-07-09 Thread Nguyễn Thái Ngọc Duy
Commit 76c61fb (log: decorate HEAD with branch name under
--decorate=full, too - 2015-05-13) adds "HEAD -> branch" decoration to
show current branch vs detached HEAD. The sign of whether HEAD is
detached or not is "->" (vs ",") because the branch is always colored
by type. Color the arrow the same as HEAD to visually emphasize that
the following branch is HEAD, without paying too much attention to the
actual separators "->" or ","

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 I'm going to take Michael's silence as no objection to the recoloring
 the arrow. So v2 changes the arrow's color instead of the branch's.

 log-tree.c   | 2 --
 t/t4207-log-decoration-colors.sh | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 78a5381..e647b08 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -263,8 +263,6 @@ void format_decorations_extended(struct strbuf *sb,
 
if (current_and_HEAD &&
decoration->type == DECORATION_REF_HEAD) {
-   strbuf_addstr(sb, color_reset);
-   strbuf_addstr(sb, color_commit);
strbuf_addstr(sb, " -> ");
strbuf_addstr(sb, color_reset);
strbuf_addstr(sb, decorate_get_color(use_color, 
current_and_HEAD->type));
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index f8008b6..b972296 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -44,7 +44,7 @@ test_expect_success setup '
 '
 
 cat >expected <\
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD ->\
  ${c_reset}${c_branch}master${c_reset}${c_commit},\
  ${c_reset}${c_tag}tag: v1.0${c_reset}${c_commit},\
  ${c_reset}${c_tag}tag: B${c_reset}${c_commit})${c_reset} B
-- 
2.8.2.537.g0965dd9

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: gc and repack ignore .git/*HEAD when checking reachability

2016-07-09 Thread Johannes Schindelin
Hi Junio,

On Fri, 8 Jul 2016, Junio C Hamano wrote:

> Josh Triplett  writes:
> 
> > That sounds reasonable.  And if they *do* end up taking any time to
> > traverse, it's because they weren't reachable from other anchoring
> > points, so taking the extra time to traverse them seems fine.
> 
> The only thing that is hard is to clearly define _what_ are the new
> anchoring points.
> 
> It cannot be "anything directly under .git that has all-caps name
> that ends with _HEAD".  The ones we write we know are going to be
> removed at some point in time (e.g. "git reset", "git bisect reset",
> "git merge --abort", etc.).  We do not have any control on random
> ones that the users and third-party tools leave behind, holding onto
> irrelevant objects forever.

Please note that bisect already uses the (transient) refs/bisect/
namespace. So I do not think we need to take specific care of the
BISECT_* files.

If we had thought of it back then, we could have used such a transient
namespace also for FETCH_HEAD, CHERRY_PICK_HEAD and also for detached
HEADs (which we should have called "unnamed branches").

Now, how about special-casing *just* these legacy files in gc: HEAD,
FETCH_HEAD, MERGE_HEAD and CHERRY_PICK_HEAD? Any new transient refs should
live in the refs/ namespace, which is already handled.

BTW this issue is getting much more problematic when you have a lot of
worktrees, some of which operate on detached HEADs. Which I do.

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] diff: fix a double off-by-one with --ignore-space-at-eol

2016-07-09 Thread Naja Melan
Thanks,

you sure are efficient in bug fixing...

good day to you
Naja Melan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git 2.8.1 - bug in patience diff algorithm when used with --ignore-space-at-eol?

2016-07-09 Thread Johannes Schindelin
Hi,

On Sat, 9 Jul 2016, Johannes Schindelin wrote:

> On Fri, 8 Jul 2016, Naja Melan wrote:
> 
> > When diffing with --patience and --ignore-space-at-eol, a change that
> > adds or removes just one character a the end of a line isn't picked up.
> 
> Confirmed with the current 'master'. I am on it [...]

And I fixed it:

http://thread.gmane.org/gmane.comp.version-control.git/299176/focus=299178

Ciao,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] diff: demonstrate a bug with --patience and --ignore-space-at-eol

2016-07-09 Thread Johannes Schindelin
When a single character is added to a line, the combination of these
two options results in an empty diff.

This bug was noticed and reported by Naja Melan.

Signed-off-by: Johannes Schindelin 
---
 t/t4033-diff-patience.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/t/t4033-diff-patience.sh b/t/t4033-diff-patience.sh
index 3c9932e..5f0d0b1 100755
--- a/t/t4033-diff-patience.sh
+++ b/t/t4033-diff-patience.sh
@@ -5,6 +5,14 @@ test_description='patience diff algorithm'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-diff-alternative.sh
 
+test_expect_failure '--ignore-space-at-eol with a single appended character' '
+   printf "a\nb\nc\n" >pre &&
+   printf "a\nbX\nc\n" >post &&
+   test_must_fail git diff --no-index \
+   --patience --ignore-space-at-eol pre post >diff &&
+   grep "^+.*X" diff
+'
+
 test_diff_frobnitz "patience"
 
 test_diff_unique "patience"
-- 
2.9.0.278.g1caae67


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] diff: fix a double off-by-one with --ignore-space-at-eol

2016-07-09 Thread Johannes Schindelin
When comparing two lines, ignoring any whitespace at the end, we first
try to match as many bytes as possible and break out of the loop only
upon mismatch, to let the remainder be handled by the code shared with
the other whitespace-ignoring code paths.

When comparing the bytes, however, we incremented the counters always,
even if the bytes did not match. And because we fall through to  the
space-at-eol handling at that point, it is as if that mismatch never
happened.

Signed-off-by: Johannes Schindelin 
---
 t/t4033-diff-patience.sh | 2 +-
 xdiff/xpatience.c| 2 +-
 xdiff/xutils.c   | 6 --
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/t/t4033-diff-patience.sh b/t/t4033-diff-patience.sh
index 5f0d0b1..113304d 100755
--- a/t/t4033-diff-patience.sh
+++ b/t/t4033-diff-patience.sh
@@ -5,7 +5,7 @@ test_description='patience diff algorithm'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-diff-alternative.sh
 
-test_expect_failure '--ignore-space-at-eol with a single appended character' '
+test_expect_success '--ignore-space-at-eol with a single appended character' '
printf "a\nb\nc\n" >pre &&
printf "a\nbX\nc\n" >post &&
test_must_fail git diff --no-index \
diff --git a/xdiff/xpatience.c b/xdiff/xpatience.c
index 04e1a1a..a613efc 100644
--- a/xdiff/xpatience.c
+++ b/xdiff/xpatience.c
@@ -1,6 +1,6 @@
 /*
  *  LibXDiff by Davide Libenzi ( File Differential Library )
- *  Copyright (C) 2003-2009 Davide Libenzi, Johannes E. Schindelin
+ *  Copyright (C) 2003-2016 Davide Libenzi, Johannes E. Schindelin
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Lesser General Public
diff --git a/xdiff/xutils.c b/xdiff/xutils.c
index 62cb23d..027192a 100644
--- a/xdiff/xutils.c
+++ b/xdiff/xutils.c
@@ -200,8 +200,10 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, 
long s2, long flags)
return 0;
}
} else if (flags & XDF_IGNORE_WHITESPACE_AT_EOL) {
-   while (i1 < s1 && i2 < s2 && l1[i1++] == l2[i2++])
-   ; /* keep going */
+   while (i1 < s1 && i2 < s2 && l1[i1] == l2[i2]) {
+   i1++;
+   i2++;
+   }
}
 
/*
-- 
2.9.0.278.g1caae67
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/2] Fix xdiff's --ignore-space-at-eol handling

2016-07-09 Thread Johannes Schindelin
It turns out that I am not the only Git developer capable of producing
an off-by-one bug ;-)

This patch series fixes a bug where we ignored single-character changes
at the end of the lines when trying to ignore white space at the end of
the lines.

I split the changes into two patches because the fix turned out to have
a much broader scope than the test (which demonstrates just a symptom):
the bug was not in the patience-specific part of the diff code, after all.


Johannes Schindelin (2):
  diff: demonstrate a bug with --patience and --ignore-space-at-eol
  diff: fix a double off-by-one with --ignore-space-at-eol

 t/t4033-diff-patience.sh | 8 
 xdiff/xpatience.c| 2 +-
 xdiff/xutils.c   | 6 --
 3 files changed, 13 insertions(+), 3 deletions(-)

Published-As: https://github.com/dscho/git/releases/tag/patience-v1
-- 
2.9.0.278.g1caae67

base-commit: 5c589a73de4394ad125a4effac227b3aec856fa1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git GUI Guesses Incorrect Language

2016-07-09 Thread Sunjay Varma
Seems like a file command issue then.

Here's the 'file' command output:

  $ file src/models/pathPlan.jsx src/services/ai.jsx
  src/models/pathPlan.jsx: C++ source, ASCII text
  src/services/ai.jsx: ASCII text

Sunjay

On Sat, Jul 9, 2016 at 2:57 AM, Johannes Sixt  wrote:
> Am 09.07.2016 um 06:57 schrieb Sunjay Varma:
>>
>> Just before the first line of my code, it says "C++ source, ASCII text".
>> That file is a JavaScript/ES6 file. The ".jsx" file extension
>> signifies that it may also contain Facebook's special JSX syntax
>> (HTML-like syntax in JavaScript).
>
>
> Git-gui just delegates to the 'file' command. What does
>
>   file src/models/pathPlan.jsx src/services/ai.jsx
>
> report? If that guesses wrongly in the same way, then the authors of the
> file command are the right address for your complaint.
>
> -- Hannes
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git GUI Guesses Incorrect Language

2016-07-09 Thread Johannes Sixt

Am 09.07.2016 um 06:57 schrieb Sunjay Varma:

Just before the first line of my code, it says "C++ source, ASCII text".
That file is a JavaScript/ES6 file. The ".jsx" file extension
signifies that it may also contain Facebook's special JSX syntax
(HTML-like syntax in JavaScript).


Git-gui just delegates to the 'file' command. What does

  file src/models/pathPlan.jsx src/services/ai.jsx

report? If that guesses wrongly in the same way, then the authors of the 
file command are the right address for your complaint.


-- Hannes

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git 2.8.1 - bug in patience diff algorithm when used with --ignore-space-at-eol?

2016-07-09 Thread Johannes Schindelin
Hi Naja,

On Fri, 8 Jul 2016, Naja Melan wrote:

> When diffing with --patience and --ignore-space-at-eol, a change that
> adds or removes just one character a the end of a line isn't picked up.

Confirmed with the current 'master'. I am on it, building on top of this
diff:

-- snipsnap --
diff --git a/t/t4033-diff-patience.sh b/t/t4033-diff-patience.sh
index 3c9932e..6da435b 100755
--- a/t/t4033-diff-patience.sh
+++ b/t/t4033-diff-patience.sh
@@ -5,6 +5,13 @@ test_description='patience diff algorithm'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-diff-alternative.sh
 
+test_expect_failure '--ignore-space-at-eol with a single appended character' '
+   printf "a\nb\nc\n" >pre &&
+   printf "a\nbX\nc\n" >post &&
+   git diff --no-index --patience --ignore-space-at-eol pre post >diff &&
+   grep "^+.*X" diff
+'
+
 test_diff_frobnitz "patience"
 
 test_diff_unique "patience"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html