Git development
 help / color / mirror / Atom feed
* [PATCH 4/6] stg mail: factor out __update_header
From: Alex Chiang @ 2009-11-28 19:50 UTC (permalink / raw)
  To: catalin.marinas; +Cc: git, Karl Wiberg
In-Reply-To: <20091128194056.949.88791.stgit@bob.kio>

Factor __update_header out of __build_address_headers.

Headers like Reply-To, Mail-Reply-To, and Mail-Followup-To are now
handled in __build_extra_headers.

We make this change because in the future, we do not want to call
__build_address_headers if using git send-email but we will always
want to call __build_extra_headers.

Cc: Karl Wiberg <kha@treskal.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 stgit/commands/mail.py |   48 +++++++++++++++++++++++++-----------------------
 1 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index fe5742e..7f811e8 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -260,25 +260,25 @@ def __send_message(tmpl, options, *args):
     out.done()
     return msg_id
 
-def __build_address_headers(msg, options, extra_cc = []):
-    """Build the address headers and check existing headers in the
-    template.
-    """
+def __update_header(msg, header, addr = '', ignore = ()):
     def __addr_pairs(msg, header, extra):
         pairs = email.Utils.getaddresses(msg.get_all(header, []) + extra)
         # remove pairs without an address and resolve the aliases
         return [address_or_alias(p) for p in pairs if p[1]]
 
-    def __update_header(header, addr = '', ignore = ()):
-        addr_pairs = __addr_pairs(msg, header, [addr])
-        del msg[header]
-        # remove the duplicates and filter the addresses
-        addr_dict = dict((addr, email.Utils.formataddr((name, addr)))
-                         for name, addr in addr_pairs if addr not in ignore)
-        if addr_dict:
-            msg[header] = ', '.join(addr_dict.itervalues())
-        return set(addr_dict.iterkeys())
+    addr_pairs = __addr_pairs(msg, header, [addr])
+    del msg[header]
+    # remove the duplicates and filter the addresses
+    addr_dict = dict((addr, email.Utils.formataddr((name, addr)))
+                     for name, addr in addr_pairs if addr not in ignore)
+    if addr_dict:
+        msg[header] = ', '.join(addr_dict.itervalues())
+    return set(addr_dict.iterkeys())
 
+def __build_address_headers(msg, options, extra_cc = []):
+    """Build the address headers and check existing headers in the
+    template.
+    """
     to_addr = ''
     cc_addr = ''
     extra_cc_addr = ''
@@ -298,18 +298,14 @@ def __build_address_headers(msg, options, extra_cc = []):
         bcc_addr = autobcc
 
     # if an address is on a header, ignore it from the rest
-    to_set = __update_header('To', to_addr)
-    cc_set = __update_header('Cc', cc_addr, to_set)
-    bcc_set = __update_header('Bcc', bcc_addr, to_set.union(cc_set))
+    to_set = __update_header(msg, 'To', to_addr)
+    cc_set = __update_header(msg, 'Cc', cc_addr, to_set)
+    bcc_set = __update_header(msg, 'Bcc', bcc_addr, to_set.union(cc_set))
 
     # --auto generated addresses, don't include the sender
-    from_set = __update_header('From')
-    __update_header('Cc', extra_cc_addr, to_set.union(bcc_set).union(from_set))
-
-    # update other address headers
-    __update_header('Reply-To')
-    __update_header('Mail-Reply-To')
-    __update_header('Mail-Followup-To')
+    from_set = __update_header(msg, 'From')
+    __update_header(msg, 'Cc', extra_cc_addr,
+                    to_set.union(bcc_set).union(from_set))
 
 def __get_signers_list(msg):
     """Return the address list generated from signed-off-by and
@@ -347,6 +343,12 @@ def __build_extra_headers(msg, msg_id, ref_id = None):
         msg['References'] = ref_id
     msg['User-Agent'] = 'StGit/%s' % version.version
 
+    # update other address headers
+    __update_header(msg, 'Reply-To')
+    __update_header(msg, 'Mail-Reply-To')
+    __update_header(msg, 'Mail-Followup-To')
+
+
 def __encode_message(msg):
     # 7 or 8 bit encoding
     charset = email.Charset.Charset('utf-8')

^ permalink raw reply related

* [PATCH 3/6] stg mail: make __send_message do more
From: Alex Chiang @ 2009-11-28 19:50 UTC (permalink / raw)
  To: catalin.marinas; +Cc: git, Karl Wiberg
In-Reply-To: <20091128194056.949.88791.stgit@bob.kio>

Factor out the common code required to send either a cover mail
or patch, and implement it in __send_message.

WRY? DRY.

Cc: Karl Wiberg <kha@treskal.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 stgit/commands/mail.py |   61 ++++++++++++++++++++----------------------------
 1 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index b6fc3d9..fe5742e 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -228,17 +228,37 @@ def __send_message_smtp(smtpserver, from_addr, to_addr_list, msg, options):
 
     s.quit()
 
-def __send_message(from_addr, to_addr_list, msg, options):
+def __send_message(tmpl, options, *args):
     """Message sending dispatcher.
     """
-    smtpserver = options.smtp_server or config.get('stgit.smtpserver')
+    msg_id = email.Utils.make_msgid('stgit')
+    build = { 1: __build_cover, 4: __build_message }
+    msg = build[len(args)](tmpl, msg_id, options, *args)
+
+    from_addr, to_addrs = __parse_addresses(msg)
+    msg_str = msg.as_string(options.mbox)
+    if options.mbox:
+        out.stdout_raw(msg_str + '\n')
+        return msg_id
+
+    outstr = { 1: 'the cover message', 4: 'patch "%s"' % args[0] }
+    out.start('Sending ' + outstr[len(args)])
 
+    smtpserver = options.smtp_server or config.get('stgit.smtpserver')
     if smtpserver.startswith('/'):
         # Use the sendmail tool
-        __send_message_sendmail(smtpserver, msg)
+        __send_message_sendmail(smtpserver, msg_str)
     else:
         # Use the SMTP server (we have host and port information)
-        __send_message_smtp(smtpserver, from_addr, to_addr_list, msg, options)
+        __send_message_smtp(smtpserver, from_addr, to_addrs, msg_str, options)
+
+    # give recipients a chance of receiving related patches in correct order
+    #                                       patch_nr < total_nr
+    if len(args) == 1 or (len(args) == 4 and args[1] < args[2]):
+        sleep = options.sleep or config.getint('stgit.smtpdelay')
+        time.sleep(sleep)
+    out.done()
+    return msg_id
 
 def __build_address_headers(msg, options, extra_cc = []):
     """Build the address headers and check existing headers in the
@@ -584,7 +604,6 @@ def func(parser, options, args):
     else:
         ref_id = None
 
-    sleep = options.sleep or config.getint('stgit.smtpdelay')
 
     # send the cover message (if any)
     if options.cover or options.edit_cover:
@@ -599,24 +618,12 @@ def func(parser, options, args):
             if not tmpl:
                 raise CmdException, 'No cover message template file found'
 
-        msg_id = email.Utils.make_msgid('stgit')
-        msg = __build_cover(tmpl, msg_id, options, patches)
-        from_addr, to_addr_list = __parse_addresses(msg)
-
-        msg_string = msg.as_string(options.mbox)
+        msg_id = __send_message(tmpl, options, patches)
 
         # subsequent e-mails are seen as replies to the first one
         if not options.noreply:
             ref_id = msg_id
 
-        if options.mbox:
-            out.stdout_raw(msg_string + '\n')
-        else:
-            out.start('Sending the cover message')
-            __send_message(from_addr, to_addr_list, msg_string, options)
-            time.sleep(sleep)
-            out.done()
-
     # send the patches
     if options.template:
         tmpl = file(options.template).read()
@@ -629,24 +636,8 @@ def func(parser, options, args):
             raise CmdException, 'No e-mail template file found'
 
     for (p, patch_nr) in zip(patches, range(1, total_nr + 1)):
-        msg_id = email.Utils.make_msgid('stgit')
-        msg = __build_message(tmpl, msg_id, options, p, patch_nr, total_nr,
-                              ref_id)
-        from_addr, to_addr_list = __parse_addresses(msg)
-
-        msg_string = msg.as_string(options.mbox)
+        msg_id = __send_message(tmpl, options, p, patch_nr, total_nr, ref_id)
 
         # subsequent e-mails are seen as replies to the first one
         if not options.noreply and not options.unrelated and not ref_id:
             ref_id = msg_id
-
-        if options.mbox:
-            out.stdout_raw(msg_string + '\n')
-        else:
-            out.start('Sending patch "%s"' % p)
-            __send_message(from_addr, to_addr_list, msg_string, options)
-            # give recipients a chance of receiving related patches in the
-            # correct order.
-            if patch_nr < total_nr:
-                time.sleep(sleep)
-            out.done()

^ permalink raw reply related

* [PATCH 5/6] stg mail: add basic support for git send-email
From: Alex Chiang @ 2009-11-28 19:50 UTC (permalink / raw)
  To: catalin.marinas; +Cc: git, Karl Wiberg
In-Reply-To: <20091128194056.949.88791.stgit@bob.kio>

This is the first step in turning stg mail into a wrapper for
git send-email. It requires passing the --git option to stg mail
for now.

Only a few basic options are supported for now, namely To/Cc/Bcc.

git send-email options used:
  --suppress-cc=self	prevent further information prompts
  --quiet		reduce git send-email output

Cc: Karl Wiberg <kha@treskal.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 stgit/commands/mail.py |   60 +++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index 7f811e8..81ec77e 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -140,7 +140,9 @@ options = [
     opt('-b', '--branch', args = [argparse.stg_branches],
         short = 'Use BRANCH instead of the default branch'),
     opt('-m', '--mbox', action = 'store_true',
-        short = 'Generate an mbox file instead of sending')
+        short = 'Generate an mbox file instead of sending'),
+    opt('--git', action = 'store_true',
+        short = 'Use git send-email (EXPERIMENTAL)')
     ] + argparse.diff_opts_option()
 
 directory = DirectoryHasRepository(log = False)
@@ -228,6 +230,52 @@ def __send_message_smtp(smtpserver, from_addr, to_addr_list, msg, options):
 
     s.quit()
 
+def __send_message_git(msg, options):
+    """Send the message using git send-email
+    """
+    from subprocess import call
+    from tempfile import mkstemp
+
+    cmd = ["git", "send-email", "--from=%s" % msg['From']]
+    cmd.append("--quiet")
+    cmd.append("--suppress-cc=self")
+
+    # XXX: yuck, there's gotta be a more pythonic way. Ideally we'd like
+    # to use the git_opts dictionary as our mapping between stg mail and
+    # git send-email; extract k, v pairs from git_opts, and use those
+    # to iterate across options somehow.
+    git_opts = { 'to': '--to=', 'cc': '--cc=', 'bcc': '--bcc=' }
+    if options.to:
+        for a in options.to:
+            cmd.append("--to=%s" % a)
+    if options.cc:
+        for a in options.cc:
+            cmd.append("--cc=%s" % a)
+    if options.bcc:
+        for a in options.bcc:
+            cmd.append("--bcc=%s" % a)
+    if not options.auto:
+        cmd.append("--suppress-cc=body")
+
+    # XXX: hack for now so that we don't duplicate To/Cc/Bcc headers
+    # in the mail, as git send-email inserts those for us.
+    del msg['To']
+    del msg['Cc']
+    del msg['Bcc']
+
+    (fd, path) = mkstemp()
+    os.write(fd, msg.as_string(options.mbox))
+    os.close(fd)
+
+    try:
+        cmd.append(path)
+        call(cmd)
+    except Exception, err:
+        os.unlink(path)
+        raise CmdException, str(err)
+
+    os.unlink(path)
+
 def __send_message(tmpl, options, *args):
     """Message sending dispatcher.
     """
@@ -242,10 +290,13 @@ def __send_message(tmpl, options, *args):
         return msg_id
 
     outstr = { 1: 'the cover message', 4: 'patch "%s"' % args[0] }
-    out.start('Sending ' + outstr[len(args)])
+    if not options.git:
+        out.start('Sending ' + outstr[len(args)])
 
     smtpserver = options.smtp_server or config.get('stgit.smtpserver')
-    if smtpserver.startswith('/'):
+    if options.git:
+        __send_message_git(msg, options)
+    elif smtpserver.startswith('/'):
         # Use the sendmail tool
         __send_message_sendmail(smtpserver, msg_str)
     else:
@@ -257,7 +308,8 @@ def __send_message(tmpl, options, *args):
     if len(args) == 1 or (len(args) == 4 and args[1] < args[2]):
         sleep = options.sleep or config.getint('stgit.smtpdelay')
         time.sleep(sleep)
-    out.done()
+    if not options.git:
+        out.done()
     return msg_id
 
 def __update_header(msg, header, addr = '', ignore = ()):

^ permalink raw reply related

* [PATCH 6/6] stg mail: don't parse To/Cc/Bcc in --git mode
From: Alex Chiang @ 2009-11-28 19:50 UTC (permalink / raw)
  To: catalin.marinas; +Cc: git, Karl Wiberg
In-Reply-To: <20091128194056.949.88791.stgit@bob.kio>

When using stg mail in --git mode, do not parse command-line To/Cc/Bcc
addresses.

Instead, we pass them directly to git send-email.

This allows us to leverage git send-email's support for email aliases.

Cc: Karl Wiberg <kha@treskal.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 stgit/commands/mail.py |   15 ++++++---------
 1 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index 81ec77e..c01e14b 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -257,12 +257,6 @@ def __send_message_git(msg, options):
     if not options.auto:
         cmd.append("--suppress-cc=body")
 
-    # XXX: hack for now so that we don't duplicate To/Cc/Bcc headers
-    # in the mail, as git send-email inserts those for us.
-    del msg['To']
-    del msg['Cc']
-    del msg['Bcc']
-
     (fd, path) = mkstemp()
     os.write(fd, msg.as_string(options.mbox))
     os.close(fd)
@@ -283,7 +277,8 @@ def __send_message(tmpl, options, *args):
     build = { 1: __build_cover, 4: __build_message }
     msg = build[len(args)](tmpl, msg_id, options, *args)
 
-    from_addr, to_addrs = __parse_addresses(msg)
+    if not options.git:
+        from_addr, to_addrs = __parse_addresses(msg)
     msg_str = msg.as_string(options.mbox)
     if options.mbox:
         out.stdout_raw(msg_str + '\n')
@@ -507,7 +502,8 @@ def __build_cover(tmpl, msg_id, options, patches):
     except Exception, ex:
         raise CmdException, 'template parsing error: %s' % str(ex)
 
-    __build_address_headers(msg, options)
+    if not options.git:
+        __build_address_headers(msg, options)
     __build_extra_headers(msg, msg_id, options.refid)
     __encode_message(msg)
 
@@ -617,7 +613,8 @@ def __build_message(tmpl, msg_id, options, patch, patch_nr, total_nr, ref_id):
     else:
         extra_cc = []
 
-    __build_address_headers(msg, options, extra_cc)
+    if not options.git:
+        __build_address_headers(msg, options, extra_cc)
     __build_extra_headers(msg, msg_id, ref_id)
     __encode_message(msg)
 

^ permalink raw reply related

* Re: non-US-ASCII file names (e.g. Hiragana) on Windows
From: Johannes Sixt @ 2009-11-28 20:00 UTC (permalink / raw)
  To: Thomas Singer; +Cc: git
In-Reply-To: <4B1168D4.5010902@syntevo.com>

On Samstag, 28. November 2009, Thomas Singer wrote:
> I've created a file with unicode characters in its name (using Java):
>
>  new File(dir, "\u3041\u3042\u3043\u3044").createNewFile();
>...
> $ git add .
> fatal: unable to stat '????': No such file or directory
>
> What should I do to make Git recognize these characters?

You cannot on a German Windows.

You can switch your Windows to Japanese (not the UI, just the codepage 
aka "locale"; yes, that's possible, I have such a setup), but even then the 
characters of the file name will be recorded in Shift-JIS encoding, not UTF-8 
or Unicode. When you later switch back to German, these bytes will be 
interpreted as cp850 or cp1252 text and displayed accordingly.

-- Hannes

^ permalink raw reply

* [RFH] Mention of 1.7.0 transition plans in Release Notes to 1.6.6
From: Junio C Hamano @ 2009-11-28 22:46 UTC (permalink / raw)
  To: git

I am attaching the first part of the current draft release notes to
request help from the developer community, so that we can avoid a fiasco
like we had at 1.6.0 when we switched from "git-foo" to "git foo".

The 1.6.0 change was widely advertised but the way it was introduced
allowed users to _ignore_ the issue until the last minute, and as the
consequence of having ignoring the issue, they were _forced_ to scramble
and adjust once their system administrator decided to install the new
version.

I think contributors who pushed for the updated safety valve for "git
push" did a lot better, learning from the bitter 1.6.0 experience, to
prevent users from ignoring the issue and instead prepare them for smooth
transition.  I wanted to make sure all the users who read Release Notes to
the upcoming release but otherwise do not follow this list regularly will
not be surprised when 1.7.0 happens, and I feel the text needs polishing
for that purpose.  We might want to deliver the announcement to a wider
audience than usual as well, and volunteers to help doing so are very
welcomed.

Two things and a half to note:

 - I do not think we have such an anti-procrastination measure for
   send-email's --[no-]chain-reply-to change.  We might want to have one
   before 1.6.6 ships; namely, if the code decided to use chain-reply-to
   behaviour by default because there was no sendemail.chainreplyto (or
   sendemail.$identity.chainreplyto) configured, nor --no-chain-reply-to
   or --chain-reply-to given from the command line, we issue a big fat
   warning just like we warn against unconfigured push.denyCurrentBranch
   when allowing pushing to a checked-out branch without being told.

 - I do not think of a sane way to cover "diff -b/-w" changes, as this is
   a "bugfix --- but there may be some scripts that have been relying on
   the bug", and a configuration option that retains the buggy behaviour
   does not make much sense.  But I may be mistaken and somebody can come
   up with an easy patch to allow both behaviour, in which case we should
   add similar anti-procrastination measure to this change.

 - Recent proposal to add "grep.fulltree" configuration variable may fall
   into the same category as sendemail.chainreplyto, as it is not about
   "buggy behaviour" nor "risky default that can easily hurt users" but is
   more about "there are two competing and equally sane defaults and it is
   purely an issue of user preference".  So if we were to decide flip the
   default in a far future, we may add such anti-procrastination measure
   there as well (but not before we decide we will flip it someday).

   This last item is obviously outside the scope of 1.7.0, though.

----------------------------------------------------------------

Git v1.6.6 Release Notes
========================

Notes on behaviour change
-------------------------

 * In this release, "git fsck" defaults to "git fsck --full" and
   checks packfiles, and because of this it will take much longer to
   complete than before.  If you prefer a quicker check only on loose
   objects (the old default), you can say "git fsck --no-full".  This
   has been supported by 1.5.4 and newer versions of git, so it is
   safe to write it in your script even if you use slightly older git
   on some of your machines.

Preparing yourselves for compatibility issues in 1.7.0
------------------------------------------------------

In git 1.7.0, which is planned to be the release after 1.6.6, there will
be a handful of behaviour changes that will break backward compatibility.

These changes were discussed long time ago and existing behaviours have
been identified as more problematic to the userbase than keeping them for
the sake of backward compatibility.

When necessary, transition strategy for existing users has been designed
not to force them running around setting configuration variables and
updating their scripts in order to keep the traditional behaviour on the
day their sysadmin decides to install the new version of git.  When we
switched from "git-foo" to "git foo" in 1.6.0, even though the change had
been advertised and the transition guide had been provided for a very long
time, the users procrastinated during the entire transtion period, and
ended up panicking on the day their sysadmins updated their git.

For changes decided to be in 1.7.0, we have been much louder to strongly
discourage such procrastination.  If you have been using recent versions
of git, you would have already seen warnings issued when you exercised
features whose behaviour will change, with the instruction on how to keep
the existing behaviour if you choose to.  You hopefully should be well
prepared already.

Of course, we have also given "this and that will change in 1.7.0; prepare
yourselves" warnings in the release notes and announcement messages.
Let's see how well users will fare this time.

 * "git push" into a branch that is currently checked out (i.e. pointed by
   HEAD in a repository that is not bare) will be refused by default.

   Similarly, "git push $there :$killed" to delete the branch $killed
   in a remote repository $there, when $killed branch is the current
   branch pointed at by its HEAD, will be refused by default.

   Setting the configuration variables receive.denyCurrentBranch and
   receive.denyDeleteCurrent to 'ignore' in the receiving repository
   can be used to override these safety features.  Versions of git
   since 1.6.2 have issued a loud warning when you tried to do them
   without setting the configuration, so repositories of people who
   still need to be able to perform such a push should already been
   future proofed.

   Please refer to:

   http://git.or.cz/gitwiki/GitFaq#non-bare
   http://thread.gmane.org/gmane.comp.version-control.git/107758/focus=108007

   for more details on the reason why this change is needed and the
   transition process that already took place so far.

 * "git send-email" will not make deep threads by default when sending a
   patch series with more than two messages.  All messages will be sent as
   a reply to the first message, i.e. cover letter.  It has been possible
   to configure send-email to do this by setting sendemail.chainreplyto
   configuration variable to false.  The only thing the new release will
   do is to change the default when you haven't configured that variable.

 * "git status" will not be "git commit --dry-run".  This change does not
   affect you if you run the command without pathspec.

   Nobody sane found the current behaviour of "git status Makefile" useful
   nor meaningful, and it confused users.  "git commit --dry-run" has been
   provided as a way to get the current behaviour of this command since
   1.6.5.

 * "git diff" traditionally treated various "ignore whitespace" options
   only as a way to filter the patch output.  "git diff --exit-code -b"
   exited with non-zero status even if all changes were about changing the
   ammount of whitespace and nothing else.  and "git diff -b" showed the
   "diff --git" header line for such a change without patch text.

   In 1.7.0, the "ignore whitespaces" will affect the semantics of the
   diff operation itself.  A change that does not affect anything but
   whitespaces will be reported with zero exit status when run with
   --exit-code, and there will not be "diff --git" header for such a
   change.

^ permalink raw reply

* Re: non-US-ASCII file names (e.g. Hiragana) on Windows
From: Maximilien Noal @ 2009-11-28 23:07 UTC (permalink / raw)
  To: Thomas Singer; +Cc: git
In-Reply-To: <4B1168D4.5010902@syntevo.com>

Thomas Singer a écrit :
> I've created a file with unicode characters in its name (using Java):
> 
>  new File(dir, "\u3041\u3042\u3043\u3044").createNewFile();
> 
> The file name is stored correctly on disk, because if invoking a
> 
>  dir.list()
> 
> the name is listed correctly.
> 
> When opening this directory in the Windows Explorer (German Windows XP SP3),
> it shows 4 boxes - which most likely is a problem of the font not supporting
> these characters.
> 
> When launching 'git status' from the git shell (msys 1.6.5.1.1367.gcd48 from
> 7zip-bundle) it only shows me 4 question marks. I would have expected to see
> the non-displayable characters escaped like it did with the umlauts on OS X.
> 
> Even adding fails:
> 
> $ git add .
> fatal: unable to stat '????': No such file or directory
> 
> What should I do to make Git recognize these characters?
> 
Hi

About the 'boxes' :

The thing is, Windows' files for Asian languages are _not_ installed by 
default.

They can be installed (even while installing Windows), by checking the 
two checkboxes under the "Supplemtal languages support" groupbox in the 
"Languages" tab of the "Regional and language options" control panel. 
*re-take some breath ;-) *

It will remove the "boxes" in Explorer and display nice Asian characters.

But that will only fix Windows' files' names display, surely not git 
(unless I'm mistaken).

^ permalink raw reply

* Re: non-US-ASCII file names (e.g. Hiragana) on Windows
From: Reece Dunn @ 2009-11-28 23:37 UTC (permalink / raw)
  To: Thomas Singer; +Cc: git
In-Reply-To: <4B1168D4.5010902@syntevo.com>

2009/11/28 Thomas Singer <thomas.singer@syntevo.com>:
>
> When launching 'git status' from the git shell (msys 1.6.5.1.1367.gcd48 from
> 7zip-bundle) it only shows me 4 question marks. I would have expected to see
> the non-displayable characters escaped like it did with the umlauts on OS X.
>
> Even adding fails:
>
> $ git add .
> fatal: unable to stat '????': No such file or directory
>
> What should I do to make Git recognize these characters?

This is a bug in git's character encoding/conversion logic. It looks
like git is taking the source string and converting it to ascii to be
displayed on the console output (e.g. by using the WideCharToMultiByte
conversion API) -- these APIs will use a '?' character for characters
that it cannot map to the target character encoding (like the Hiragana
characters that you are using).

SetConsoleOutputCP can be used to change the console output codepage
[http://msdn.microsoft.com/en-us/library/ms686036%28VS.85%29.aspx] and
SetConsoleCP is the equivalent for input
[http://msdn.microsoft.com/en-us/library/ms686013%28VS.85%29.aspx].
e.g.

    SetConsoleCP(CP_UTF8);
    SetConsoleOutputCP(CP_UTF8);

should make the console process UTF-8 characters, so git shouldn't
need to do any character conversions on Windows when reading/writing
it's data.

NOTE: I have not tested this, just noting what I have found via Google.

- Reece

^ permalink raw reply

* Re: [PATCH 3/4] build dashless "test-bin" directory similar to installed bindir
From: Junio C Hamano @ 2009-11-29  2:32 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Matthew Ogilvie, git
In-Reply-To: <20091128194910.GA17605@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Sat, Nov 28, 2009 at 11:44:39AM -0800, Junio C Hamano wrote:
>
>>    . Advertising this forces the Makefile build test-bin/ contents from
>>      "all" target.  I think test-bin/ should only depend on "test" (iow,
>>      after "make all && make install" there shouldn't have to be "test-bin"
>>      directory.
>
> Would implementing it that way mean that:
>
>   make && cd t && make
>
> does not work (or worse, might silently use stale information in
> test-bin)?

Why can't t/Makefile have a dependency on its 'default' target that goes
up and prepares test-bin/, i.e. "cd .. && make test-bin-stuff"?

^ permalink raw reply

* Re: [PATCH 2/2] Add gitk-git Hungarian translation
From: Laszlo Papp @ 2009-11-29  2:33 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Laszlo Papp, git
In-Reply-To: <a362e8010911220005u1783cd44yf84ae5bc5b42d980@mail.gmail.com>

On Sun, Nov 22, 2009 at 9:05 AM, Laszlo Papp <djszapi@archlinux.us> wrote:
> On Thu, Nov 19, 2009 at 10:07 AM, Paul Mackerras <paulus@samba.org> wrote:
>> Laszlo Papp writes:
>>
>>> Signed-off-by: Laszlo Papp <djszapi@archlinux.us>
>>> ---
>>>  gitk-git/po/hu.po | 1151 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>
>> Is there a patch 1/2 that needs to be applied first, as the subject
>> line might imply?
>>
>> Paul.
>>
>
> It's the file of the new hungarian translation, it can be applied
> alone without any plus file/patch.
>
> Best Regards,
> Laszlo Papp
>

Up!

^ permalink raw reply

* Re: [PATCH 3/4] build dashless "test-bin" directory similar to installed bindir
From: Matthew Ogilvie @ 2009-11-29  2:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, peff
In-Reply-To: <7vtywefn88.fsf@alter.siamese.dyndns.org>

On Sat, Nov 28, 2009 at 11:44:39AM -0800, Junio C Hamano wrote:
>  - Patch #1 and #2 are good and are independent from the later patches, as
>    without them running tests with GIT_TEST_INSTALLED would not work.
> 
>    By the way, 6720721 (test-lib.sh: Allow running the test suite against
>    installed git, 2009-03-16) failed to document the feature in t/README.
>    Could you please fix this while you are at it?

Sure, I'll include another patch for this when I re-roll the
series.  It will probably mention something about still needing
a build so that it can access the test-* support executables.

>  - It certainly is _possible_ to add $(pwd)/test-bin to $PATH instead of
>    the established practice of using GIT_EXEC_PATH for every day/permanent
>    use without installation, but I doubt we should _encourage_ it for a
>    few reasons:
> 
>    . The set-up will force one extra exec due to the wrapper; this is good
>      for the purpose of running tests, but unnecessary for a set-up for
>      every day/permanent use by people, compared with the already working
>      approach.  The user needs to change an environment variable _anyway_
>      (either GIT_EXEC_PATH with the traditional approach, or PATH with
>      your patch).
> 
>    . The new component to be added to $PATH shouldn't be named "test-bin/"
>      if it is meant for every day/permanent use.
> 
>    . Advertising this forces the Makefile build test-bin/ contents from
>      "all" target.  I think test-bin/ should only depend on "test" (iow,
>      after "make all && make install" there shouldn't have to be "test-bin"
>      directory.
> 
>    I would rather treat it an unintended side-effect that you can add that
>    directory to the $PATH.  It is designed to work in such an environment
>    (otherwise the tests won't exercise the version of git they are meant
>    to test), but I do not think it is _meant_ to be _used_  by end users
>    that way.  And an unintended side-effect does not have to be mentioned
>    in INSTALL (especially with the directory name with "test" in it).

I personally like the idea of being able to use an uninstalled
build without touching environment variables at all.  Just specify
the full path to the the version you want to run on the
command line, as in: ~/SANDBOX/test-bin/git WHATEVER
Especially handy for trying "ssh MACHINE /PATH/SANDBOX/...".

FYI: There are already a number of test suite support executables
built by "make all" (test-*).  It might be hard to eliminate them
from "all" without risking stale executables.  As Jeff
King <peff@peff.net> pointed out in a separate email, some people
(including me) often don't use the top-level "make test" target
to run tests.

I'm still thinking about this.  I've noted some possible changes
to the patch series below, some of which are mutually exclusive.
Any opinions?

Options geared towards isolating/hiding test-bin:

  1. Scrap the part of the patch that modifies INSTALL.

  2. Perhaps use hardlinks, symlinks, and/or copies within test-bin,
     instead of wrapper scripts, to eliminate the extra exec.  Since
     test-lib.sh already sets up necessary environment variables,
     they don't strictly need to be set in the wrappers as
     well.  On the other hand, hardlinks and copies are potentially
     vulnerable to stale executable issues, and symlinks typically
     don't work on Windows.  

  3. Scrap pre-built test-bin completely, and switch to a solution
     that more closely resembles the valgrind option (have test-lib.sh
     build the directory).  This can't use the same makefile variables
     to define the contents of the directory, though.

Options geared towards making something like test-bin an official way
to run an uninstalled build:

  4. Rename test-bin.  Perhaps "bin-wrappers", "bin-dashless",
     "bin-install", "bin", or "bindir".  Any preferences?

  5. The current patch doesn't quite handle the simple
     "~/SANDBOX/test-bin/gitSOMETHING WHATEVER" idiom perfectly
     if the executable (gitSOMETHING) tries to run additional
     git commands without adjusting the PATH first.  I could enhance
     the wrapper to prefix test-bin onto the PATH just in case it
     isn't there already.

Other cleanup options:

  6. There is a stale script issue if someone does something like:
       make
       cp -a . /some/other/path
       cd /some/other/path
       [optional modifications, without a "make clean"]
       make
       [run tests; uses wrong executables...]
     Including GIT-CFLAGS as a makefile dependency for the
     wrappers was intended to address this, but looking
     closer, I don't think it works.  Perhaps I should
     include $(shell pwd) in GIT-CFLAGS, or make a new GIT-PWD target
     that works similarly to GIT-CFLAGS.  Without this, a workaround
     (and probably best option overall) is to do a "make clean" after
     copying a sandbox.

  7. Enable similar dashless environment when
     GIT_TEST_INSTALLED and/or valgrind are enabled?

  8. Include wrappers for other dashed-commands in test-bin, which
     would always fail, in case someone runs tests with an installed
     GIT_EXEC_PATH already in their PATH.  This might catch a new test
     using dashes in such an environment.  I don't really think this
     is worth it, though.  Most people don't have GIT_EXEC_PATH in their
     PATH, and some such person would notice any problems soon.

  9. This may be outside the scope of this patch series, but perhaps
     git executables could try to find argv[0] in the PATH
     (if argv[0] is not absolute), and see if they can find various other
     executables (GIT_EXEC_PATH) and data files (perl, templates,
     etc) using paths relative to itself.  This may include
     manually dereferencing argv[0] if it is a symlink.  GIT_EXEC_PATH
     and friends still takes precedence, but only fallback on
     compile-time defaults if "find relative to argv[0]" fails.
     It looks like Makefile RUNTIME_PREFIX enables something like
     this, but it is currently disabled by default on most platforms.

--
Matthew Ogilvie   [mmogilvi_git@miniinfo.net]

^ permalink raw reply

* Re: [RFH] Mention of 1.7.0 transition plans in Release Notes to 1.6.6
From: Nanako Shiraishi @ 2009-11-29  3:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Michael Witten, Jay Soffian
In-Reply-To: <7vfx7yfetb.fsf@alter.siamese.dyndns.org>

Subject: prepare send-email for smoother change of --chain-reply-to default

Give a warning message when send-email uses chain-reply-to to thread the
messages because of the current default, not because the user explicitly
asked to, either from the command line or from the configuration.

This way, by the time 1.7.0 switches the default, everybody will be ready.

Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
---
 Quoting Junio C Hamano <gitster@pobox.com>

 >  - I do not think we have such an anti-procrastination measure for
 >    send-email's --[no-]chain-reply-to change.  We might want to have one
 >    before 1.6.6 ships; namely, if the code decided to use chain-reply-to
 >    behaviour by default because there was no sendemail.chainreplyto (or
 >    sendemail.$identity.chainreplyto) configured, nor --no-chain-reply-to
 >    or --chain-reply-to given from the command line, we issue a big fat
 >    warning just like we warn against unconfigured push.denyCurrentBranch
 >    when allowing pushing to a checked-out branch without being told.

 Like this?

 git-send-email.perl   |   19 +++++++++++++++++--
 t/t9001-send-email.sh |   40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 4f5da4e..2afed76 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -187,9 +187,11 @@ my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);
 my ($validate, $confirm);
 my (@suppress_cc);
 
+my $not_set_by_user = "true but not set by the user";
+
 my %config_bool_settings = (
     "thread" => [\$thread, 1],
-    "chainreplyto" => [\$chain_reply_to, 1],
+    "chainreplyto" => [\$chain_reply_to, $not_set_by_user],
     "suppressfrom" => [\$suppress_from, undef],
     "signedoffbycc" => [\$signed_off_by_cc, undef],
     "signedoffcc" => [\$signed_off_by_cc, undef],      # Deprecated
@@ -214,6 +216,19 @@ my %config_settings = (
     "from" => \$sender,
 );
 
+# Help users prepare for 1.7.0
+sub chain_reply_to {
+	if (defined $chain_reply_to &&
+	    $chain_reply_to eq $not_set_by_user) {
+		print STDERR
+		    "In git 1.7.0, the default will be changed to --no-chain-reply-to\n" .
+		    "Set sendemail.chainreplyto configuration variable to true if\n" .
+		    "you want to keep --chain-reply-to as your default.\n";
+		$chain_reply_to = 1;
+	}
+	return $chain_reply_to;
+}
+
 # Handle Uncouth Termination
 sub signal_handler {
 
@@ -1157,7 +1172,7 @@ foreach my $t (@files) {
 
 	# set up for the next message
 	if ($thread && $message_was_sent &&
-		($chain_reply_to || !defined $reply_to || length($reply_to) == 0)) {
+		(chain_reply_to() || !defined $reply_to || length($reply_to) == 0)) {
 		$reply_to = $message_id;
 		if (length $references > 0) {
 			$references .= "\n $message_id";
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 84a7f03..4372774 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -769,4 +769,44 @@ test_expect_success 'threading but no chain-reply-to' '
 	grep "In-Reply-To: " stdout
 '
 
+test_expect_success 'no warning with an explicit --chain-reply-to' '
+	git send-email \
+	--dry-run \
+	--from="Example <nobody@example.com>" \
+	--to=nobody@example.com \
+	--chain-reply-to \
+	outdir/000?-*.patch 2>errors >out &&
+	! grep "no-chain-reply-to" errors
+'
+
+test_expect_success 'no warning with an explicit --no-chain-reply-to' '
+	git send-email \
+	--dry-run \
+	--from="Example <nobody@example.com>" \
+	--to=nobody@example.com \
+	--no-chain-reply-to \
+	outdir/000?-*.patch 2>errors >out &&
+	! grep "no-chain-reply-to" errors
+'
+
+test_expect_success 'no warning with sendemail.chainreplyto = false' '
+	git config sendemail.chainreplyto false &&
+	git send-email \
+	--dry-run \
+	--from="Example <nobody@example.com>" \
+	--to=nobody@example.com \
+	outdir/000?-*.patch 2>errors >out &&
+	! grep "no-chain-reply-to" errors
+'
+
+test_expect_success 'no warning with sendemail.chainreplyto = true' '
+	git config sendemail.chainreplyto true &&
+	git send-email \
+	--dry-run \
+	--from="Example <nobody@example.com>" \
+	--to=nobody@example.com \
+	outdir/000?-*.patch 2>errors >out &&
+	! grep "no-chain-reply-to" errors
+'
+
 test_done


-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply related

* "git merge" merges too much!
From: Greg A. Woods @ 2009-11-29  3:21 UTC (permalink / raw)
  To: The Git Mailing List

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

I'm trying to learn to use Git to manage local changes.  I'm very new to
Git (hi all!), but not new at all to version tracking tools in general.

Hope I've found the right list on which to ask potentially naive
questions!  I've been doing _lots_ of reading about Git, but I can't
seem to find anything about the problems I relate below.

One task I'm working on is to try to find the best way to merge changes
made from one branch to another, eg. to propagate local fixes from one
release to another.

However in at least one simple case "git merge" merges too much.

I have something like this started from a remote-cloned repository where
BL1.2 is a branch from the remote master HEAD, which happens to
correspond to a tag "TR1.2", the release-1.2 tag, and I've made three
local commits to my local BL1.2 branch: A, B, and C:

                                         BL1.2 - A - B - C  <- BL1.2 HEAD
                                        /
master 1 - 2 - TR1.1 - 3 - 4 - 5 - TR1.2  <- master HEAD


(there are no "release" branches in this project, just tags on the
master branch to represent release points -- is there any way to get
"git log" to show which tags are associated with a given commit and/or
branch?  The real project is freedesktop.org's xinit repo, but the real
tree is too messy to diagram here -- hopefully I've extracted the
essence of the problem correctly)

I now want to create a branch "BL1.1" and merge commits A, B, and C to it
in order to back-port my local fixes to the TR1.1 release.  "TR1.1" is
simply a tag on the origin/master trunk.

I do the following:

	git checkout -b BL1.1 TR1.1
	git merge BL1.2

However this seems to merge all of 3, 4, and 5, as well as A, B, and C.

I think I can (barely) understand why it's doing what it's doing, but
that's not what I want it to do.  However it looks like Git doesn't have
the same idea of a branch "base" point as I think I do.

Running "git log TR1.2..BL1.2" does show me exactly the changes I wish
to propagate, but "git merge TR1.2..BL1.2" says "not something we can
merge".  Sigh.

How can I get it to merge just the changes from the "base" of the BL1.2
branch to its head?

Is using either git-cherry-pick or "git log -p | git-am", the only way
to do this?  Which way best preserves Git's ability to realize if a
change has already been included on the target branch, if any?

Is this the kind of "problem" that drove the creators of Stacked-Git to
invent their tools?

Is there any way to get "git log --graph" (and/or gitk) to show me all
the branch heads, not just the current/specified one?

-- 
						Greg A. Woods

+1 416 218-0098                VE3TCP          RoboHack <woods@robohack.ca>
Planix, Inc. <woods@planix.com>      Secrets of the Weird <woods@weird.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 186 bytes --]

^ permalink raw reply

* Re: [PATCH 3/4] build dashless "test-bin" directory similar to installed bindir
From: Jeff King @ 2009-11-29  3:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Matthew Ogilvie, git
In-Reply-To: <7vaay6f4ce.fsf@alter.siamese.dyndns.org>

On Sat, Nov 28, 2009 at 06:32:33PM -0800, Junio C Hamano wrote:

> > Would implementing it that way mean that:
> >
> >   make && cd t && make
> >
> > does not work (or worse, might silently use stale information in
> > test-bin)?
> 
> Why can't t/Makefile have a dependency on its 'default' target that goes
> up and prepares test-bin/, i.e. "cd .. && make test-bin-stuff"?

Yeah, that would work (I really should have phrased my other response
less as a critique and more as "please don't break this workflow"). But
I don't think the default target would be enough. I would also expect

  make && cd t && make tXXXX-YYYY.sh

to work correctly (and to be pedantic, I am actually more interested in
the equivalent situation that one has one window looking at code and
compiling, and another window running a test script, but they are
functionally equivalent here).

I also like to be able to simply run ./tXXXX-YYYY.sh. I can accept
losing that if there is something to be gained, but if we can keep it, I
suspect I am not the only one who uses it.

-Peff

^ permalink raw reply

* Re: [PATCH 3/4] build dashless "test-bin" directory similar to installed bindir
From: Matthew Ogilvie @ 2009-11-29  4:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vaay6f4ce.fsf@alter.siamese.dyndns.org>

On Sat, Nov 28, 2009 at 06:32:33PM -0800, Junio C Hamano wrote:
> Why can't t/Makefile have a dependency on its 'default' target that goes
> up and prepares test-bin/, i.e. "cd .. && make test-bin-stuff"?

That wouldn't help with someone manually running specific tests,
which is a common way to run tests when working on git:

   hackhackhack && make && cd t && ./tXXXX-*.sh

Perhaps we could invoke make from test-lib.sh, but at that point
it starts to look more attractive to just do it the same way as the
valgrind option, even though that means duplicating the appropriate
list of executables in test-lib.sh.

Also, "make all" already builds some test suite support
binaries (test-*).  Is it really worth any effort to leave
a few short wrapper scripts out of "make all" when it is already
building several test binaries?

(See also my much longer email response that crossed this one in the
mail.)

--
Matthew Ogilvie   [mmogilvi_git@miniinfo.net]

^ permalink raw reply

* Re: [PATCH 3/4] build dashless "test-bin" directory similar to installed bindir
From: Junio C Hamano @ 2009-11-29  5:07 UTC (permalink / raw)
  To: Jeff King; +Cc: Matthew Ogilvie, git
In-Reply-To: <20091129034313.GA28379@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

>   make && cd t && make tXXXX-YYYY.sh
>
> to work correctly ...
>
> I also like to be able to simply run ./tXXXX-YYYY.sh.

I think both can be done by running "cd .. && make test-bin-stuff" from
test-lib.sh if you wanted to.  Isn't it essentially what you do for
valgrind?

^ permalink raw reply

* Re: [PATCH 3/4] build dashless "test-bin" directory similar to installed bindir
From: Jeff King @ 2009-11-29  5:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Matthew Ogilvie, git
In-Reply-To: <7vzl65ex6u.fsf@alter.siamese.dyndns.org>

On Sat, Nov 28, 2009 at 09:07:05PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> >   make && cd t && make tXXXX-YYYY.sh
> >
> > to work correctly ...
> >
> > I also like to be able to simply run ./tXXXX-YYYY.sh.
> 
> I think both can be done by running "cd .. && make test-bin-stuff" from
> test-lib.sh if you wanted to.  Isn't it essentially what you do for
> valgrind?

Yes, except that the valgrind setup doesn't actually interact with the
Makefile; it just assumes that git-* is interesting. I would be happy
with a solution that is triggered from test-lib. It may even be possible
to refactor the valgrind stuff, but I'm not sure. What Dscho eventually
submitted doesn't bear much resemblence to my original attempt, and I've
forgotten all of the details.

-Peff

^ permalink raw reply

* Re: "git merge" merges too much!
From: Jeff King @ 2009-11-29  5:14 UTC (permalink / raw)
  To: The Git Mailing List
In-Reply-To: <m1NEaLp-000kn1C@most.weird.com>

On Sat, Nov 28, 2009 at 10:21:25PM -0500, Greg A. Woods wrote:

> Hope I've found the right list on which to ask potentially naive
> questions!  I've been doing _lots_ of reading about Git, but I can't
> seem to find anything about the problems I relate below.

Yep, you're in the right place.

> master branch to represent release points -- is there any way to get
> "git log" to show which tags are associated with a given commit and/or

Try "git log --decorate".

>                                          BL1.2 - A - B - C  <- BL1.2 HEAD
>                                         /
> master 1 - 2 - TR1.1 - 3 - 4 - 5 - TR1.2  <- master HEAD
>
> [...]
> 
> 	git checkout -b BL1.1 TR1.1
> 	git merge BL1.2
> 
> However this seems to merge all of 3, 4, and 5, as well as A, B, and C.
> 
> I think I can (barely) understand why it's doing what it's doing, but
> that's not what I want it to do.  However it looks like Git doesn't have
> the same idea of a branch "base" point as I think I do.

Yes. Git doesn't really view history as branches in the way you are
thinking. History is simply a directed graph, and when you merge two
nodes in the graph, it takes into account _everything_ that happened
to reach those two points since the last time they diverged (which in
your case is simply TR1.1, as BL1.2 is a strict superset).

There is no way in the history graph to represent "we have these
commits, but not this subsequence". You have to create new commits A',
B', and C' which introduce more or less the same changes as their
counterparts (and they may even be _exactly_ the same except for the
parentage, but then again, they may not if the changes they make do not
apply in the same way on top of TR1.1).

> Running "git log TR1.2..BL1.2" does show me exactly the changes I wish
> to propagate, but "git merge TR1.2..BL1.2" says "not something we can
> merge".  Sigh.
> 
> How can I get it to merge just the changes from the "base" of the BL1.2
> branch to its head?
> 
> Is using either git-cherry-pick or "git log -p | git-am", the only way
> to do this?  Which way best preserves Git's ability to realize if a
> change has already been included on the target branch, if any?

Yes, you must cherry-pick or use rebase (which is a more featureful
version of the pipeline you mentioned). Either way will produce an
equivalent set of commits (cherry-pick is useful when you are picking a
couple of commits; rebase is useful for rewriting a whole stretch of
history. It sounds like you want to do the latter).

The resulting commits will have different commit ids, but git generally
does a good job at merging such things, because it looks only at the
result state and not the intermediate commits.  If both sides have made
an equivalent change, then there is no conflict.

> Is there any way to get "git log --graph" (and/or gitk) to show me all
> the branch heads, not just the current/specified one?

Try "--all" with either gitk or "git log". Or if you want a subset of
heads, just name them.

Hope that helps,
-Peff

^ permalink raw reply

* Re: "git merge" merges too much!
From: Junio C Hamano @ 2009-11-29  5:15 UTC (permalink / raw)
  To: Greg A. Woods; +Cc: The Git Mailing List
In-Reply-To: <m1NEaLp-000kn1C@most.weird.com>

In order to make things smoother and easier in the future, you may want to
learn "topic branch" workflows (found in many git tutorial material).

But it is too late for the history you already created; "cherry-pick" is
your friend to recover from the shape of your existing history.

^ permalink raw reply

* Re: Unable to checkout a particular SVN revision
From: Marc Liyanage @ 2009-11-29  6:41 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git
In-Reply-To: <4B113BAD.8090604@drmicha.warpmail.net>


Thanks for the explanation so far.

On 28.11.2009, at 07:03, Michael J Gruber wrote:

> No. Because "this" is different in the two cases above: "git svn clone"
> clones the history of an svn repo, and the command above clearly gives
> you the history of that branch in the specified revision range
> (consisting of 1 revision). It is empty.

I still don't quite understand why it couldn't do the same thing as the SVN checkout. That does exactly what I expect, it reflects the state of that part of the repository at the time of that revision. Would this be possible, but it's simply not (yet) implemented?

The problem I'm dealing with are svn:externals definitions that are pegged to such revisions. SVN checks them out fine, but git svn doesn't, so I have to hunt down the appropriate revision manually.




______________________________
Marc Liyanage
www.entropy.ch

skype mliyanage
iChat liyanage@mac.com

^ permalink raw reply

* Re: [RFH] Mention of 1.7.0 transition plans in Release Notes to 1.6.6
From: Junio C Hamano @ 2009-11-29  7:24 UTC (permalink / raw)
  To: git
In-Reply-To: <7vfx7yfetb.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

>  - I do not think of a sane way to cover "diff -b/-w" changes, as this is
>    a "bugfix --- but there may be some scripts that have been relying on
>    the bug", and a configuration option that retains the buggy behaviour
>    does not make much sense.  But I may be mistaken and somebody can come
>    up with an easy patch to allow both behaviour, in which case we should
>    add similar anti-procrastination measure to this change.

The patch to enable/disable this feature would look like the attached,
which doesn't look too bad.  This apply to a merge between two topics:

    jc/1.7.0-diff-whitespace-only-status (97bf2a0)
    gb/1.7.0-diff-whitespace-only-output (3e97c7c)

An anti-procrastination measure must be done in at least two, but probably
in three steps on top of this patch:

 #1. Flip the default in this patch back to 1, as the traditional
     behaviour is to treat ignore-whitespace options as purely affecting
     output of patch text, not affecting status nor the header;

 #2. Add extra logic to detect if a particular invocation may trigger a
     different behaviour once the default is changed to 0, and issue an
     warning if there is no configuration explicitly set;

 #3. Flip the default to 0, perhaps still keeping the warning when there
     is no configuration.

Make a major release after the second step is done, and wait for at least
one but preferrably two releases, and then ship the result of the third
step in the "flag day" release.  In a much later version we would remove
the "no configuration warning".

Do we need an anti-procrastination measure for this?  If so, I think we
probably need to postpone the "diff" semantics changes after 1.7.0, as I
am not very confident that we can solidly finish the second step before
the 1.6.6 final.

Also, I am not very keen on having this configuration, as its only purpose
is to ask for a broken semantics from "diff", even though it has been the
traditional behaviour for the past four years.

Comments?

---
 diff.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index da90e6e..6e7c47c 100644
--- a/diff.c
+++ b/diff.c
@@ -29,6 +29,7 @@ static const char *diff_word_regex_cfg;
 static const char *external_diff_cmd_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
+static int diff_b_w_output_only;
 
 static char diff_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_RESET,
@@ -133,6 +134,11 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
+	if (!strcmp(var, "diff.bwoutputonly")) {
+		diff_b_w_output_only = git_config_bool(var, value);
+		return 0;
+	}
+
 	return git_color_default_config(var, value, cb);
 }
 
@@ -1668,7 +1674,7 @@ static void builtin_diff(const char *name_a,
 		struct emit_callback ecbdata;
 		const struct userdiff_funcname *pe;
 
-		if (!DIFF_XDL_TST(o, WHITESPACE_FLAGS)) {
+		if (!DIFF_XDL_TST(o, WHITESPACE_FLAGS) || diff_b_w_output_only) {
 			fprintf(o->file, "%s", header.buf);
 			strbuf_reset(&header);
 		}
@@ -2537,9 +2543,10 @@ int diff_setup_done(struct diff_options *options)
 	 * inside contents.
 	 */
 
-	if (DIFF_XDL_TST(options, IGNORE_WHITESPACE) ||
-	    DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) ||
-	    DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL))
+	if (!diff_b_w_output_only &&
+	    (DIFF_XDL_TST(options, IGNORE_WHITESPACE) ||
+	     DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) ||
+	     DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL)))
 		DIFF_OPT_SET(options, DIFF_FROM_CONTENTS);
 	else
 		DIFF_OPT_CLR(options, DIFF_FROM_CONTENTS);
-- 
1.6.6.rc0.61.g41d5b.dirty

^ permalink raw reply related

* [PATCH 1/3] diff: flip the default diff.bwoutputonly to true
From: Junio C Hamano @ 2009-11-29  7:25 UTC (permalink / raw)
  To: git
In-Reply-To: <7vmy25dc9h.fsf@alter.siamese.dyndns.org>

This switches the default behaviour of "diff -w/-b" back to the
traditional "ignore whitespace affects only output of patch text, not
status nor diff header", and allows the planned new semantics to be asked
in advance by setting diff.bwoutputonly configuration to false.

Also a loud warning is issued when the configuration is not set at all,
so that we can flip the default later more smoothly.

Update tests to check cases where the configuration is set to true, false
and not set at all; make sure the warning is issued when and only when
necessary.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c                       |   23 ++++++++++---
 t/t4015-diff-whitespace.sh   |   70 +++++++++++++++++++++++++++++++++---------
 t/t4040-whitespace-status.sh |   48 ++++++++++++++++++----------
 3 files changed, 103 insertions(+), 38 deletions(-)

diff --git a/diff.c b/diff.c
index 6e7c47c..fb93a22 100644
--- a/diff.c
+++ b/diff.c
@@ -29,7 +29,8 @@ static const char *diff_word_regex_cfg;
 static const char *external_diff_cmd_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
-static int diff_b_w_output_only;
+static int diff_b_w_output_only = 1;
+static int diff_b_w_output_only_given;
 
 static char diff_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_RESET,
@@ -135,6 +136,7 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
 	}
 
 	if (!strcmp(var, "diff.bwoutputonly")) {
+		diff_b_w_output_only_given = 1;
 		diff_b_w_output_only = git_config_bool(var, value);
 		return 0;
 	}
@@ -2521,9 +2523,15 @@ void diff_setup(struct diff_options *options)
 	}
 }
 
+static const char *bw_option_warning =
+	"ignore-whitespace options will stop showing diff headers in later\n"
+	"versions of git; set diff.bwoutputonly to true to keep the old\n"
+	"behaviour, or set.bwoutputonly to false to squelch this message.\n";
+
 int diff_setup_done(struct diff_options *options)
 {
 	int count = 0;
+	int bw_options;
 
 	if (options->output_format & DIFF_FORMAT_NAME)
 		count++;
@@ -2542,11 +2550,14 @@ int diff_setup_done(struct diff_options *options)
 	 * --ignore-whitespace* options force us to look
 	 * inside contents.
 	 */
-
-	if (!diff_b_w_output_only &&
-	    (DIFF_XDL_TST(options, IGNORE_WHITESPACE) ||
-	     DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) ||
-	     DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL)))
+	bw_options = (DIFF_XDL_TST(options, IGNORE_WHITESPACE) ||
+		      DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) ||
+		      DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL));
+	if (!diff_b_w_output_only_given && bw_options && bw_option_warning) {
+		warning("%s", bw_option_warning);
+		bw_option_warning = NULL;
+	}
+	if (!diff_b_w_output_only && bw_options)
 		DIFF_OPT_SET(options, DIFF_FROM_CONTENTS);
 	else
 		DIFF_OPT_CLR(options, DIFF_FROM_CONTENTS);
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 90f3342..310421b 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -92,16 +92,30 @@ EOF
 git diff > out
 test_expect_success 'another test, without options' 'test_cmp expect out'
 
-cat << EOF > expect
+test_w_b () {
+	note=$1
+	for o in "-w" "-w -b" "-w --ignore-space-at-eol" "-w -b --ignore-space-at-eol"
+	do
+		test_expect_success "with $o ($note)" '
+			git diff $o >actual &&
+			test_cmp expect actual
+		'
+	done
+}
+
+git config diff.bwoutputonly false
+> expect
+test_w_b "bwoutputonly=false"
+
+git config diff.bwoutputonly true
+cat <<EOF >expect
+diff --git a/x b/x
+index d99af23..8b32fb5 100644
 EOF
-git diff -w > out
-test_expect_success 'another test, with -w' 'test_cmp expect out'
-git diff -w -b > out
-test_expect_success 'another test, with -w -b' 'test_cmp expect out'
-git diff -w --ignore-space-at-eol > out
-test_expect_success 'another test, with -w --ignore-space-at-eol' 'test_cmp expect out'
-git diff -w -b --ignore-space-at-eol > out
-test_expect_success 'another test, with -w -b --ignore-space-at-eol' 'test_cmp expect out'
+test_w_b "bwoutputonly=true"
+
+git config --unset diff.bwoutputonly
+test_w_b "bwoutputonly unset"
 
 tr 'Q' '\015' << EOF > expect
 diff --git a/x b/x
@@ -384,20 +398,47 @@ test_expect_success 'checkdiff allows new blank lines' '
 	git diff --check
 '
 
-cat <<EOF >expect
-EOF
-test_expect_success 'whitespace-only changes not reported' '
+test_expect_success 'whitespace-only changes shown' '
+	git config --unset diff.bwoutputonly
 	git reset --hard &&
 	echo >x "hello world" &&
 	git add x &&
 	git commit -m "hello 1" &&
 	echo >x "hello  world" &&
 	git diff -b >actual &&
-	test_cmp expect actual
+	test 2 = $(wc -l <actual)
 '
 
-test_expect_success 'combined diff with autocrlf conversion' '
+test_expect_success 'whitespace-only changes shown with diff.bwoutputonly' '
+	git config diff.bwoutputonly true &&
+	git diff -b >actual &&
+	test 2 = $(wc -l <actual)
+'
+
+test_expect_success 'whitespace-only changes hidden with !diff.bwoutputonly' '
+	git config diff.bwoutputonly false &&
+	git diff -b >actual &&
+	! test -s actual
+'
+
+test_expect_success 'no warning with diff.bwoutputonly given' '
+	git diff -b 2>errors &&
+	! grep warn errors
+'
 
+test_expect_success 'no warning without diff.bwoutputonly' '
+	git config --unset diff.bwoutputonly
+	git diff -p 2>errors &&
+	! grep "stop showing" errors
+'
+
+test_expect_success 'warning without diff.bwoutputonly' '
+	git config --unset diff.bwoutputonly
+	git diff -b 2>errors &&
+	grep "stop showing" errors
+'
+
+test_expect_success 'combined diff with autocrlf conversion' '
 	git reset --hard &&
 	echo >x hello &&
 	git commit -m "one side" x &&
@@ -409,7 +450,6 @@ test_expect_success 'combined diff with autocrlf conversion' '
 
 	git diff | sed -e "1,/^@@@/d" >actual &&
 	! grep "^-" actual
-
 '
 
 test_done
diff --git a/t/t4040-whitespace-status.sh b/t/t4040-whitespace-status.sh
index a30b03b..95a93f7 100755
--- a/t/t4040-whitespace-status.sh
+++ b/t/t4040-whitespace-status.sh
@@ -19,45 +19,59 @@ test_expect_success setup '
 	git add a/d
 '
 
-test_expect_success 'diff-tree --exit-code' '
+test_once () {
+	note=$1 tmf=$2
+
+test_expect_success "diff-tree --exit-code ($note)" '
 	test_must_fail git diff --exit-code HEAD^ HEAD &&
 	test_must_fail git diff-tree --exit-code HEAD^ HEAD
 '
 
-test_expect_success 'diff-tree -b --exit-code' '
-	git diff -b --exit-code HEAD^ HEAD &&
-	git diff-tree -b -p --exit-code HEAD^ HEAD &&
-	git diff-tree -b --exit-code HEAD^ HEAD
+test_expect_success "diff-tree -b --exit-code ($note)" '
+	$tmf git diff -b --exit-code HEAD^ HEAD &&
+	$tmf git diff-tree -b -p --exit-code HEAD^ HEAD &&
+	$tmf git diff-tree -b --exit-code HEAD^ HEAD
 '
 
-test_expect_success 'diff-index --cached --exit-code' '
+test_expect_success "diff-index --cached --exit-code ($note)" '
 	test_must_fail git diff --cached --exit-code HEAD &&
 	test_must_fail git diff-index --cached --exit-code HEAD
 '
 
-test_expect_success 'diff-index -b -p --cached --exit-code' '
-	git diff -b --cached --exit-code HEAD &&
-	git diff-index -b -p --cached --exit-code HEAD
+test_expect_success "diff-index -b -p --cached --exit-code ($note)" '
+	$tmf git diff -b --cached --exit-code HEAD &&
+	$tmf git diff-index -b -p --cached --exit-code HEAD
 '
 
-test_expect_success 'diff-index --exit-code' '
+test_expect_success "diff-index --exit-code ($note)" '
 	test_must_fail git diff --exit-code HEAD &&
 	test_must_fail git diff-index --exit-code HEAD
 '
 
-test_expect_success 'diff-index -b -p --exit-code' '
-	git diff -b --exit-code HEAD &&
-	git diff-index -b -p --exit-code HEAD
+test_expect_success "diff-index -b -p --exit-code ($note)" '
+	$tmf git diff -b --exit-code HEAD &&
+	$tmf git diff-index -b -p --exit-code HEAD
 '
 
-test_expect_success 'diff-files --exit-code' '
+test_expect_success "diff-files --exit-code ($note)" '
 	test_must_fail git diff --exit-code &&
 	test_must_fail git diff-files --exit-code
 '
 
-test_expect_success 'diff-files -b -p --exit-code' '
-	git diff -b --exit-code &&
-	git diff-files -b -p --exit-code
+test_expect_success "diff-files -b -p --exit-code ($note)" '
+	$tmf git diff -b --exit-code &&
+	$tmf git diff-files -b -p --exit-code
 '
 
+}
+
+git config diff.bwoutputonly true
+test_once "bwoutputonly=true" "test_must_fail"
+
+git config diff.bwoutputonly false
+test_once "bwoutputonly=false" ""
+
+git config --unset diff.bwoutputonly
+test_once "bwoutputonly unset" "test_must_fail"
+
 test_done
-- 
1.6.6.rc0.61.g41d5b.dirty

^ permalink raw reply related

* [PATCH 2/3] diff: flip the diff.bwoutputonly default to false
From: Junio C Hamano @ 2009-11-29  7:25 UTC (permalink / raw)
  To: git
In-Reply-To: <7vmy25dc9h.fsf@alter.siamese.dyndns.org>

This finally corrects the broken "ignore whitespace options only
affect patch output and never affects status nor header output",
as we have been planning for 1.7.0

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c                       |    2 +-
 t/t4015-diff-whitespace.sh   |    5 +++--
 t/t4040-whitespace-status.sh |    2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index fb93a22..df67f18 100644
--- a/diff.c
+++ b/diff.c
@@ -29,7 +29,7 @@ static const char *diff_word_regex_cfg;
 static const char *external_diff_cmd_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
-static int diff_b_w_output_only = 1;
+static int diff_b_w_output_only;
 static int diff_b_w_output_only_given;
 
 static char diff_colors[][COLOR_MAXLEN] = {
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 310421b..8ca81e8 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -114,6 +114,7 @@ index d99af23..8b32fb5 100644
 EOF
 test_w_b "bwoutputonly=true"
 
+>expect
 git config --unset diff.bwoutputonly
 test_w_b "bwoutputonly unset"
 
@@ -398,7 +399,7 @@ test_expect_success 'checkdiff allows new blank lines' '
 	git diff --check
 '
 
-test_expect_success 'whitespace-only changes shown' '
+test_expect_success 'whitespace-only changes hidden' '
 	git config --unset diff.bwoutputonly
 	git reset --hard &&
 	echo >x "hello world" &&
@@ -406,7 +407,7 @@ test_expect_success 'whitespace-only changes shown' '
 	git commit -m "hello 1" &&
 	echo >x "hello  world" &&
 	git diff -b >actual &&
-	test 2 = $(wc -l <actual)
+	! test -s actual
 '
 
 test_expect_success 'whitespace-only changes shown with diff.bwoutputonly' '
diff --git a/t/t4040-whitespace-status.sh b/t/t4040-whitespace-status.sh
index 95a93f7..57db3cc 100755
--- a/t/t4040-whitespace-status.sh
+++ b/t/t4040-whitespace-status.sh
@@ -72,6 +72,6 @@ git config diff.bwoutputonly false
 test_once "bwoutputonly=false" ""
 
 git config --unset diff.bwoutputonly
-test_once "bwoutputonly unset" "test_must_fail"
+test_once "bwoutputonly unset" ""
 
 test_done
-- 
1.6.6.rc0.61.g41d5b.dirty

^ permalink raw reply related

* [PATCH 3/3] diff: disable diff.bwoutputonly warning
From: Junio C Hamano @ 2009-11-29  7:26 UTC (permalink / raw)
  To: git
In-Reply-To: <7vmy25dc9h.fsf@alter.siamese.dyndns.org>

After 1.7.0 (or whatever version) ships and everybody expects the new
semantics from "diff", we can squelch the warning.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c                     |   11 -----------
 t/t4015-diff-whitespace.sh |    4 ++--
 2 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/diff.c b/diff.c
index df67f18..ba1f482 100644
--- a/diff.c
+++ b/diff.c
@@ -30,7 +30,6 @@ static const char *external_diff_cmd_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_b_w_output_only;
-static int diff_b_w_output_only_given;
 
 static char diff_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_RESET,
@@ -136,7 +135,6 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
 	}
 
 	if (!strcmp(var, "diff.bwoutputonly")) {
-		diff_b_w_output_only_given = 1;
 		diff_b_w_output_only = git_config_bool(var, value);
 		return 0;
 	}
@@ -2523,11 +2521,6 @@ void diff_setup(struct diff_options *options)
 	}
 }
 
-static const char *bw_option_warning =
-	"ignore-whitespace options will stop showing diff headers in later\n"
-	"versions of git; set diff.bwoutputonly to true to keep the old\n"
-	"behaviour, or set.bwoutputonly to false to squelch this message.\n";
-
 int diff_setup_done(struct diff_options *options)
 {
 	int count = 0;
@@ -2553,10 +2546,6 @@ int diff_setup_done(struct diff_options *options)
 	bw_options = (DIFF_XDL_TST(options, IGNORE_WHITESPACE) ||
 		      DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) ||
 		      DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL));
-	if (!diff_b_w_output_only_given && bw_options && bw_option_warning) {
-		warning("%s", bw_option_warning);
-		bw_option_warning = NULL;
-	}
 	if (!diff_b_w_output_only && bw_options)
 		DIFF_OPT_SET(options, DIFF_FROM_CONTENTS);
 	else
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 8ca81e8..0964ea2 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -433,10 +433,10 @@ test_expect_success 'no warning without diff.bwoutputonly' '
 	! grep "stop showing" errors
 '
 
-test_expect_success 'warning without diff.bwoutputonly' '
+test_expect_success 'no warning anymore' '
 	git config --unset diff.bwoutputonly
 	git diff -b 2>errors &&
-	grep "stop showing" errors
+	! grep "stop showing" errors
 '
 
 test_expect_success 'combined diff with autocrlf conversion' '
-- 
1.6.6.rc0.61.g41d5b.dirty

^ permalink raw reply related

* Re: git-svn: SVK merge commits can have >2 parents
From: Alex Vandiver @ 2009-11-29  7:46 UTC (permalink / raw)
  To: Sam Vilain, Eric Wong; +Cc: git
In-Reply-To: <1259479636-sup-573@utwig>

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

At Sun Nov 29 02:28:39 -0500 2009, Alex Vandiver wrote:
> While converting a mildly complicated svn repository that was managed
> with SVK, I ran across the following oddness.  `svk smerge` can only
> merge between _two_ branches at once -- however, the way that svk
> merge detection works, you can end up with erroneous extra parents
> from long-dead branches.

Upon a little more inspection, I now understand that the rev-parse
lines in find_extra_svk_parents are attempting to deal with this exact
circumstance -- but they fail to properly sort the merge tickets
first, which leads to this incorrect behavior.  Armed with this
understanding, I'm more confident in the attached updated patch.  I
assume, however, that the logic allows for more than one extra parent
only because such an occurrance could be constructed by hand-editing
svk:merge, because AFAIK svk's command-line tools should be able to
construct such a circumstance.
 - Alex
-- 
Networking -- only one letter away from not working

[-- Attachment #2: 0001-git-svn-sort-svk-merge-tickets-to-account-for-minima.patch --]
[-- Type: application/octet-stream, Size: 1369 bytes --]

From 4d30e57e5da7c2e880908bc742cf80990d6f9f5d Mon Sep 17 00:00:00 2001
From: Alex Vandiver <alex@chmrr.net>
Date: Sun, 29 Nov 2009 02:20:21 -0500
Subject: [PATCH] git-svn: sort svk merge tickets to account for minimal parents

When merging branches based on svk:merge properties, a single merge
can have updated or added multiple svk:merge lines.  Attempt to
include the minimal set of parents by sorting the merge properties in
order of revision, highest to lowest.

Signed-off-by: Alex Vandiver <alex@chmrr.net>
---
 git-svn.perl |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 957d44e..51f03ad 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2940,10 +2940,14 @@ sub find_extra_svk_parents {
 			if ( my $commit = $gs->rev_map_get($rev, $uuid) ) {
 				# wahey!  we found it, but it might be
 				# an old one (!)
-				push @known_parents, $commit;
+				push @known_parents, [ $rev, $commit ];
 			}
 		}
 	}
+	# Ordering matters; highest-numbered commit merge tickets
+	# first, as they may account for later merge ticket additions
+	# or changes.
+	@known_parents = map {$_->[1]} sort {$b->[0] <=> $a->[0]} @known_parents;
 	for my $parent ( @known_parents ) {
 		my @cmd = ('rev-list', $parent, map { "^$_" } @$parents );
 		my ($msg_fh, $ctx) = command_output_pipe(@cmd);
-- 
1.6.6.rc0.254.g7352d


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox