* [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 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 2/6] stg mail: reorder __build_[message|cover] parameters
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>
Reorder the argument lists for both __build_cover and __build_message.
This change will aid readability of a subsequent refactoring patch.
Cc: Karl Wiberg <kha@treskal.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---
stgit/commands/mail.py | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index 3978f5e..b6fc3d9 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -370,7 +370,7 @@ def __edit_message(msg):
return msg
-def __build_cover(tmpl, patches, msg_id, options):
+def __build_cover(tmpl, msg_id, options, patches):
"""Build the cover message (series description) to be sent via SMTP
"""
sender = __get_sender()
@@ -439,7 +439,7 @@ def __build_cover(tmpl, patches, msg_id, options):
return msg
-def __build_message(tmpl, patch, patch_nr, total_nr, msg_id, ref_id, options):
+def __build_message(tmpl, msg_id, options, patch, patch_nr, total_nr, ref_id):
"""Build the message to be sent via SMTP
"""
p = crt_series.get_patch(patch)
@@ -600,7 +600,7 @@ def func(parser, options, args):
raise CmdException, 'No cover message template file found'
msg_id = email.Utils.make_msgid('stgit')
- msg = __build_cover(tmpl, patches, msg_id, options)
+ msg = __build_cover(tmpl, msg_id, options, patches)
from_addr, to_addr_list = __parse_addresses(msg)
msg_string = msg.as_string(options.mbox)
@@ -630,8 +630,8 @@ def func(parser, options, args):
for (p, patch_nr) in zip(patches, range(1, total_nr + 1)):
msg_id = email.Utils.make_msgid('stgit')
- msg = __build_message(tmpl, p, patch_nr, total_nr, msg_id, ref_id,
- options)
+ 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)
^ permalink raw reply related
* [PATCH 1/6] stg mail: Refactor __send_message and friends
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>
Instead of passing all the various smtp* args to __send_message
individually, let's just pass the options list instead.
The main motivation is for future patches. The end goal is to
thin out stg mail's implementation and make it a minimal wrapper
around git send-email. By passing the options list to __send_message
we prepare to pass options directly to git send-email.
As a bonus, this change results in a cleaner internal API.
Finally, it also pushes the smtp logic where it belongs, viz. into
__send_message_smtp, instead of cluttering up the main body of
mail.func().
Cc: Karl Wiberg <kha@treskal.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---
stgit/commands/mail.py | 43 +++++++++++++++++++------------------------
1 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index abd42e4..3978f5e 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -190,10 +190,20 @@ def __send_message_sendmail(sendmail, msg):
cmd = sendmail.split()
Run(*cmd).raw_input(msg).discard_output()
-def __send_message_smtp(smtpserver, from_addr, to_addr_list, msg,
- smtpuser, smtppassword, use_tls):
+def __send_message_smtp(smtpserver, from_addr, to_addr_list, msg, options):
"""Send the message using the given SMTP server
"""
+ smtppassword = options.smtp_password or config.get('stgit.smtppassword')
+ smtpuser = options.smtp_user or config.get('stgit.smtpuser')
+ smtpusetls = options.smtp_tls or config.get('stgit.smtptls') == 'yes'
+
+ if (smtppassword and not smtpuser):
+ raise CmdException, 'SMTP password supplied, username needed'
+ if (smtpusetls and not smtpuser):
+ raise CmdException, 'SMTP over TLS requested, username needed'
+ if (smtpuser and not smtppassword):
+ smtppassword = getpass.getpass("Please enter SMTP password: ")
+
try:
s = smtplib.SMTP(smtpserver)
except Exception, err:
@@ -203,7 +213,7 @@ def __send_message_smtp(smtpserver, from_addr, to_addr_list, msg,
try:
if smtpuser and smtppassword:
s.ehlo()
- if use_tls:
+ if smtpusetls:
if not hasattr(socket, 'ssl'):
raise CmdException, "cannot use TLS - no SSL support in Python"
s.starttls()
@@ -218,17 +228,17 @@ def __send_message_smtp(smtpserver, from_addr, to_addr_list, msg,
s.quit()
-def __send_message(smtpserver, from_addr, to_addr_list, msg,
- smtpuser, smtppassword, use_tls):
+def __send_message(from_addr, to_addr_list, msg, options):
"""Message sending dispatcher.
"""
+ smtpserver = options.smtp_server or config.get('stgit.smtpserver')
+
if smtpserver.startswith('/'):
# Use the sendmail tool
__send_message_sendmail(smtpserver, msg)
else:
# Use the SMTP server (we have host and port information)
- __send_message_smtp(smtpserver, from_addr, to_addr_list, msg,
- smtpuser, smtppassword, use_tls)
+ __send_message_smtp(smtpserver, from_addr, to_addr_list, msg, options)
def __build_address_headers(msg, options, extra_cc = []):
"""Build the address headers and check existing headers in the
@@ -543,8 +553,6 @@ def func(parser, options, args):
"""Send the patches by e-mail using the patchmail.tmpl file as
a template
"""
- smtpserver = options.smtp_server or config.get('stgit.smtpserver')
-
applied = crt_series.get_applied()
if options.all:
@@ -564,17 +572,6 @@ def func(parser, options, args):
raise CmdException, 'Cannot send empty patch "%s"' % p
out.done()
- smtppassword = options.smtp_password or config.get('stgit.smtppassword')
- smtpuser = options.smtp_user or config.get('stgit.smtpuser')
- smtpusetls = options.smtp_tls or config.get('stgit.smtptls') == 'yes'
-
- if (smtppassword and not smtpuser):
- raise CmdException, 'SMTP password supplied, username needed'
- if (smtpusetls and not smtpuser):
- raise CmdException, 'SMTP over TLS requested, username needed'
- if (smtpuser and not smtppassword):
- smtppassword = getpass.getpass("Please enter SMTP password: ")
-
total_nr = len(patches)
if total_nr == 0:
raise CmdException, 'No patches to send'
@@ -616,8 +613,7 @@ def func(parser, options, args):
out.stdout_raw(msg_string + '\n')
else:
out.start('Sending the cover message')
- __send_message(smtpserver, from_addr, to_addr_list, msg_string,
- smtpuser, smtppassword, smtpusetls)
+ __send_message(from_addr, to_addr_list, msg_string, options)
time.sleep(sleep)
out.done()
@@ -648,8 +644,7 @@ def func(parser, options, args):
out.stdout_raw(msg_string + '\n')
else:
out.start('Sending patch "%s"' % p)
- __send_message(smtpserver, from_addr, to_addr_list, msg_string,
- smtpuser, smtppassword, smtpusetls)
+ __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:
^ permalink raw reply related
* [StGit RFC PATCH 0/6] add support for git send-email
From: Alex Chiang @ 2009-11-28 19:50 UTC (permalink / raw)
To: catalin.marinas; +Cc: git, Karl Wiberg
Hi Catalin,
This series starts down the path of eventually converting stg mail to
simply become a wrapper around git send-email.
The first 4 patches do some refactoring of internal APIs to set us up
for the money patch 5/6 which actually adds the call to git send-email.
Patch 6/6 shows a hint of the future, where we can start leveraging
some of the features in git send-email for our own purposes. With it,
you can now use mail aliases as supported by git send-email.
stg mail still has some nice features over git send-email, such
as the -v command line parameter and --prefix. Maybe at some point
in the future, we can migrate those features into git send-email and
continue thinning out stg mail.
But I wanted to get some feedback first to make sure I'm going in the
right direction before going too much further.
Disclaimer: I'm not really a python coder. Particularly, patch 5/6
is ugly in how we look at the various stg mail options. I'm sure there's
lots of room for improvement.
This mail was sent with:
./stg mail -a --git --auto -e
[Karl, sorry about the earlier accidental mail. That was a testing mishap.]
Thanks,
/ac
---
Alex Chiang (6):
stg mail: Refactor __send_message and friends
stg mail: reorder __build_[message|cover] parameters
stg mail: make __send_message do more
stg mail: factor out __update_header
stg mail: add basic support for git send-email
stg mail: don't parse To/Cc/Bcc in --git mode
stgit/commands/mail.py | 205 ++++++++++++++++++++++++++++--------------------
1 files changed, 121 insertions(+), 84 deletions(-)
^ permalink raw reply
* Re: [PATCH 3/4] build dashless "test-bin" directory similar to installed bindir
From: Jeff King @ 2009-11-28 19:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Matthew Ogilvie, git
In-Reply-To: <7vtywefn88.fsf@alter.siamese.dyndns.org>
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)? Dealing with this is part of the reason the valgrind code
(which similarly sets up a pseudo-installed directory) does everything
in test-lib.sh, and not as part of the make process.
-Peff
^ permalink raw reply
* Re: [PATCH v2 0/3] format-patch: improve pathspec usage
From: Junio C Hamano @ 2009-11-28 19:47 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <1259408429-5685-1-git-send-email-felipe.contreras@gmail.com>
Thanks; I think I have queued the equivalent already and pushed out last
night.
I think making the full-diff default is a regression without an ability to
turn it off and is a bit premature without a solid discussion in the
documentation.
^ permalink raw reply
* Re: [PATCH 3/4] build dashless "test-bin" directory similar to installed bindir
From: Junio C Hamano @ 2009-11-28 19:44 UTC (permalink / raw)
To: Matthew Ogilvie; +Cc: git
In-Reply-To: <1259433537-3963-4-git-send-email-mmogilvi_git@miniinfo.net>
Matthew Ogilvie <mmogilvi_git@miniinfo.net> writes:
> The new test-bin directory contains wrapper scripts for executables that
> will be installed into the standard bindir. It explicitly does not
> contain most dashed-commands. The scripts automatically set environment
> variables to run out of the source tree, not the installed directory.
Even though I haven't tried the series, I like most of the things I see in
this series.
- 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?
- Running tests before installation in an environment slightly closer to
the final installation (i.e. lacks dashed commands in the $PATH during
the time tests are run) is a good direction to go.
- I like the Makefile changes that uses these new BINDIR_PROGRAMS_NEED_X,
TEST_PROGRAMS_NEED_X. Parameterizing commands listed in the actions
part is good.
- 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).
^ permalink raw reply
* [PATCH 3/4] build dashless "test-bin" directory similar to installed bindir
From: Matthew Ogilvie @ 2009-11-28 18:38 UTC (permalink / raw)
To: git, gitster; +Cc: Matthew Ogilvie
In-Reply-To: <1259433537-3963-3-git-send-email-mmogilvi_git@miniinfo.net>
The new test-bin directory contains wrapper scripts
for executables that will be installed into the standard
bindir. It explicitly does not contain most dashed-commands.
The scripts automatically set environment variables to run out
of the source tree, not the installed directory.
This will allow running the test suite without dashed commands in
the PATH, and provides a simplified way to run custom built git
executables without installing them first.
test-bin also contains wrappers for some test suite support executables,
where the test suite will soon make use of them.
Signed-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
---
.gitignore | 1 +
INSTALL | 8 +++++++-
Makefile | 46 +++++++++++++++++++++++++++++++++-------------
test-bin-wrapper.sh | 13 +++++++++++++
4 files changed, 54 insertions(+), 14 deletions(-)
create mode 100644 test-bin-wrapper.sh
diff --git a/.gitignore b/.gitignore
index ac02a58..c981e4c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,6 +10,7 @@
/git-apply
/git-archimport
/git-archive
+/test-bin/
/git-bisect
/git-bisect--helper
/git-blame
diff --git a/INSTALL b/INSTALL
index be504c9..d092d33 100644
--- a/INSTALL
+++ b/INSTALL
@@ -39,7 +39,13 @@ Issues of note:
with --disable-transition option to avoid this.
- You can use git after building but without installing if you
- wanted to. Various git commands need to find other git
+ want to.
+
+ The simplest option is to use the wrapper scripts that are built
+ and saved into `pwd`/test-bin, by either putting test-bin in your
+ PATH, or invoking the scripts in test-bin using their full paths.
+
+ Alternatively, various git commands need to find other git
commands and scripts to do their work, so you would need to
arrange a few environment variables to tell them that their
friends will be found in your built source area instead of at
diff --git a/Makefile b/Makefile
index 5a0b3d4..e153ff0 100644
--- a/Makefile
+++ b/Makefile
@@ -416,6 +416,15 @@ ALL_PROGRAMS = $(PROGRAMS) $(SCRIPTS)
# what 'all' will build but not install in gitexecdir
OTHER_PROGRAMS = git$X
+# what test wrappers are needed and 'install' will install, in bindir
+BINDIR_PROGRAMS_NEED_X += git
+BINDIR_PROGRAMS_NEED_X += git-upload-pack
+BINDIR_PROGRAMS_NEED_X += git-receive-pack
+BINDIR_PROGRAMS_NEED_X += git-upload-archive
+BINDIR_PROGRAMS_NEED_X += git-shell
+
+BINDIR_PROGRAMS_NO_X += git-cvsserver
+
# Set paths to tools early so that they can be used for version tests.
ifndef SHELL_PATH
SHELL_PATH = /bin/sh
@@ -1690,19 +1699,27 @@ endif
### Testing rules
-TEST_PROGRAMS += test-chmtime$X
-TEST_PROGRAMS += test-ctype$X
-TEST_PROGRAMS += test-date$X
-TEST_PROGRAMS += test-delta$X
-TEST_PROGRAMS += test-dump-cache-tree$X
-TEST_PROGRAMS += test-genrandom$X
-TEST_PROGRAMS += test-match-trees$X
-TEST_PROGRAMS += test-parse-options$X
-TEST_PROGRAMS += test-path-utils$X
-TEST_PROGRAMS += test-sha1$X
-TEST_PROGRAMS += test-sigchain$X
+TEST_PROGRAMS_NEED_X += test-chmtime
+TEST_PROGRAMS_NEED_X += test-ctype
+TEST_PROGRAMS_NEED_X += test-date
+TEST_PROGRAMS_NEED_X += test-delta
+TEST_PROGRAMS_NEED_X += test-dump-cache-tree
+TEST_PROGRAMS_NEED_X += test-genrandom
+TEST_PROGRAMS_NEED_X += test-match-trees
+TEST_PROGRAMS_NEED_X += test-parse-options
+TEST_PROGRAMS_NEED_X += test-path-utils
+TEST_PROGRAMS_NEED_X += test-sha1
+TEST_PROGRAMS_NEED_X += test-sigchain
+
+TEST_PROGRAMS = $(patsubst %,%$X,$(TEST_PROGRAMS_NEED_X))
-all:: $(TEST_PROGRAMS)
+test_bindir_programs := $(patsubst %,test-bin/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))
+
+all:: $(TEST_PROGRAMS) $(test_bindir_programs)
+
+test-bin/%: test-bin-wrapper.sh GIT-CFLAGS
+ @mkdir -p test-bin
+ $(QUIET_GEN)sed -e 's|__GIT_EXEC_PATH__|$(shell pwd)|' -e 's|__PROG__|$(@F)|' < $< > $@ && chmod +x $@
# GNU make supports exporting all variables by "export" without parameters.
# However, the environment gets quite big, and some programs have problems
@@ -1763,11 +1780,13 @@ endif
gitexec_instdir_SQ = $(subst ','\'',$(gitexec_instdir))
export gitexec_instdir
+install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) $(BINDIR_PROGRAMS_NO_X)
+
install: all
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
- $(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X git-shell$X git-cvsserver '$(DESTDIR_SQ)$(bindir_SQ)'
+ $(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
ifndef NO_PERL
$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
@@ -1878,6 +1897,7 @@ clean:
$(LIB_FILE) $(XDIFF_LIB)
$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
$(RM) $(TEST_PROGRAMS)
+ $(RM) -r test-bin
$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*
$(RM) -r autom4te.cache
$(RM) config.log config.mak.autogen config.mak.append config.status config.cache
diff --git a/test-bin-wrapper.sh b/test-bin-wrapper.sh
new file mode 100644
index 0000000..a01eee1
--- /dev/null
+++ b/test-bin-wrapper.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+# test-bin-wrapper.sh: Template for git executable wrapper scripts
+# to run test suite against sandbox, but with only bindir-installed
+# executables in PATH. The Makefile copies this into various
+# files in test-bin, substituting __GIT_EXEC_PATH__ and __PROG__.
+
+GIT_EXEC_PATH="__GIT_EXEC_PATH__"
+GIT_TEMPLATE_DIR="__GIT_EXEC_PATH__/templates/blt"
+GITPERLLIB="__GIT_EXEC_PATH__/perl/blib/lib"
+export GIT_EXEC_PATH GIT_TEMPLATE_DIR GITPERLLIB
+
+exec "${GIT_EXEC_PATH}/__PROG__" "$@"
--
1.6.4.GIT
^ permalink raw reply related
* [PATCH 1/4] t2300: use documented technique to invoke git-sh-setup
From: Matthew Ogilvie @ 2009-11-28 18:38 UTC (permalink / raw)
To: git, gitster; +Cc: Matthew Ogilvie
In-Reply-To: <1259433537-3963-1-git-send-email-mmogilvi_git@miniinfo.net>
This is needed to allow the test suite to run against a standard
install bin directory instead of GIT_EXEC_PATH.
Signed-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
---
t/t2300-cd-to-toplevel.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t2300-cd-to-toplevel.sh b/t/t2300-cd-to-toplevel.sh
index 3b01ad2..9965bc5 100755
--- a/t/t2300-cd-to-toplevel.sh
+++ b/t/t2300-cd-to-toplevel.sh
@@ -8,7 +8,7 @@ test_cd_to_toplevel () {
test_expect_success $3 "$2" '
(
cd '"'$1'"' &&
- . git-sh-setup &&
+ . "$(git --exec-path)"/git-sh-setup &&
cd_to_toplevel &&
[ "$(pwd -P)" = "$TOPLEVEL" ]
)
--
1.6.4.GIT
^ permalink raw reply related
* [PATCH 4/4] run test suite without dashed git-commands in PATH
From: Matthew Ogilvie @ 2009-11-28 18:38 UTC (permalink / raw)
To: git, gitster; +Cc: Matthew Ogilvie
In-Reply-To: <1259433537-3963-4-git-send-email-mmogilvi_git@miniinfo.net>
Only put test-bin in the PATH (do not put GIT_EXEC_PATH in the PATH),
to emulate the default installed user environment, and
ensure all the programs run correctly in such an
environment. This is now the default, although
it can be overridden with a --with-dashes test option when running
tests.
Signed-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
---
t/README | 8 ++++++++
t/test-lib.sh | 33 +++++++++++++++++++++------------
2 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/t/README b/t/README
index d8f6c7d..b61c34b 100644
--- a/t/README
+++ b/t/README
@@ -75,6 +75,14 @@ appropriately before running "make".
As the names depend on the tests' file names, it is safe to
run the tests with this option in parallel.
+--with-dashes::
+ By default tests are run without dashed forms of
+ commands (like git-commit) in the PATH (it only uses
+ wrappers from TOP/git-bin). Use this option to include TOP
+ in the PATH, which conains all the dashed forms of commands.
+ This option is currently implied by other options like --valgrind
+ and GIT_TEST_INSTALLED.
+
Skipping Tests
--------------
diff --git a/t/test-lib.sh b/t/test-lib.sh
index ec3336a..54570ac 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -105,6 +105,8 @@ do
verbose=t; shift ;;
-q|--q|--qu|--qui|--quie|--quiet)
quiet=t; shift ;;
+ --with-dashes)
+ with_dashes=t; shift ;;
--no-color)
color=; shift ;;
--no-python)
@@ -551,19 +553,8 @@ test_done () {
# Test the binaries we have just built. The tests are kept in
# t/ subdirectory and are run in 'trash directory' subdirectory.
TEST_DIRECTORY=$(pwd)
-if test -z "$valgrind"
+if test -n "$valgrind"
then
- if test -z "$GIT_TEST_INSTALLED"
- then
- PATH=$TEST_DIRECTORY/..:$PATH
- GIT_EXEC_PATH=$TEST_DIRECTORY/..
- else
- GIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path) ||
- error "Cannot run git from $GIT_TEST_INSTALLED."
- PATH=$GIT_TEST_INSTALLED:$TEST_DIRECTORY/..:$PATH
- GIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}
- fi
-else
make_symlink () {
test -h "$2" &&
test "$1" = "$(readlink "$2")" || {
@@ -625,6 +616,24 @@ else
PATH=$GIT_VALGRIND/bin:$PATH
GIT_EXEC_PATH=$GIT_VALGRIND/bin
export GIT_VALGRIND
+elif test -n "$GIT_TEST_INSTALLED" ; then
+ GIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path) ||
+ error "Cannot run git from $GIT_TEST_INSTALLED."
+ PATH=$GIT_TEST_INSTALLED:$TEST_DIRECTORY/..:$PATH
+ GIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}
+else # normal case, use ../test-bin only unless $with_dashes:
+ git_bin_dir="$TEST_DIRECTORY/../test-bin"
+ if ! test -x "$git_bin_dir/git" ; then
+ if test -z "$with_dashes" ; then
+ say "$git_bin_dir/git is not executable; using GIT_EXEC_PATH"
+ fi
+ with_dashes=t
+ fi
+ PATH="$git_bin_dir:$PATH"
+ GIT_EXEC_PATH=$TEST_DIRECTORY/..
+ if test -n "$with_dashes" ; then
+ PATH="$TEST_DIRECTORY/..:$PATH"
+ fi
fi
GIT_TEMPLATE_DIR=$(pwd)/../templates/blt
unset GIT_CONFIG
--
1.6.4.GIT
^ permalink raw reply related
* [PATCH 2/4] t3409 t4107 t7406: use dashless commands
From: Matthew Ogilvie @ 2009-11-28 18:38 UTC (permalink / raw)
To: git, gitster; +Cc: Matthew Ogilvie
In-Reply-To: <1259433537-3963-2-git-send-email-mmogilvi_git@miniinfo.net>
This is needed to allow test suite to run against a standard
install bin directory instead of GIT_EXEC_PATH.
Signed-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
---
t/t3409-rebase-preserve-merges.sh | 6 +++---
t/t4107-apply-ignore-whitespace.sh | 20 ++++++++++----------
t/t7406-submodule-update.sh | 4 ++--
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 297d165..8f785e7 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -32,14 +32,14 @@ export GIT_AUTHOR_EMAIL
test_expect_success 'setup for merge-preserving rebase' \
'echo First > A &&
git add A &&
- git-commit -m "Add A1" &&
+ git commit -m "Add A1" &&
git checkout -b topic &&
echo Second > B &&
git add B &&
- git-commit -m "Add B1" &&
+ git commit -m "Add B1" &&
git checkout -f master &&
echo Third >> A &&
- git-commit -a -m "Modify A2" &&
+ git commit -a -m "Modify A2" &&
git clone ./. clone1 &&
cd clone1 &&
diff --git a/t/t4107-apply-ignore-whitespace.sh b/t/t4107-apply-ignore-whitespace.sh
index 484654d..b04fc8f 100755
--- a/t/t4107-apply-ignore-whitespace.sh
+++ b/t/t4107-apply-ignore-whitespace.sh
@@ -136,37 +136,37 @@ void print_int(int num) {
EOF
test_expect_success 'file creation' '
- git-apply patch1.patch
+ git apply patch1.patch
'
test_expect_success 'patch2 fails (retab)' '
- test_must_fail git-apply patch2.patch
+ test_must_fail git apply patch2.patch
'
test_expect_success 'patch2 applies with --ignore-whitespace' '
- git-apply --ignore-whitespace patch2.patch
+ git apply --ignore-whitespace patch2.patch
'
test_expect_success 'patch2 reverse applies with --ignore-space-change' '
- git-apply -R --ignore-space-change patch2.patch
+ git apply -R --ignore-space-change patch2.patch
'
git config apply.ignorewhitespace change
test_expect_success 'patch2 applies (apply.ignorewhitespace = change)' '
- git-apply patch2.patch
+ git apply patch2.patch
'
test_expect_success 'patch3 fails (missing string at EOL)' '
- test_must_fail git-apply patch3.patch
+ test_must_fail git apply patch3.patch
'
test_expect_success 'patch4 fails (missing EOL at EOF)' '
- test_must_fail git-apply patch4.patch
+ test_must_fail git apply patch4.patch
'
test_expect_success 'patch5 applies (leading whitespace)' '
- git-apply patch5.patch
+ git apply patch5.patch
'
test_expect_success 'patches do not mangle whitespace' '
@@ -175,11 +175,11 @@ test_expect_success 'patches do not mangle whitespace' '
test_expect_success 're-create file (with --ignore-whitespace)' '
rm -f main.c &&
- git-apply patch1.patch
+ git apply patch1.patch
'
test_expect_success 'patch5 fails (--no-ignore-whitespace)' '
- test_must_fail git-apply --no-ignore-whitespace patch5.patch
+ test_must_fail git apply --no-ignore-whitespace patch5.patch
'
test_done
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 2d33d9e..8e2449d 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -14,8 +14,8 @@ submodule and "git submodule update --rebase/--merge" does not detach the HEAD.
compare_head()
{
- sha_master=`git-rev-list --max-count=1 master`
- sha_head=`git-rev-list --max-count=1 HEAD`
+ sha_master=`git rev-list --max-count=1 master`
+ sha_head=`git rev-list --max-count=1 HEAD`
test "$sha_master" = "$sha_head"
}
--
1.6.4.GIT
^ permalink raw reply related
* [PATCH 0/4] Run test suite without dashed commands in PATH
From: Matthew Ogilvie @ 2009-11-28 18:38 UTC (permalink / raw)
To: git, gitster; +Cc: Matthew Ogilvie
This patch series runs the test suite without the dashed commands
in the PATH. This improves the usefulness of the test suite
as examples of how to do things, and more importantly,
minimizes any possibility of regressions in bindir-installed
scripts that might prevent them from working in a standard
install where most dashed commands are not in the PATH (git-cvsserver
was broken in this way until Sep 2009: d2feb01aa5).
The scripts in the "test-bin" directory that patch 3 creates
can also be useful for manually testing a build of git without
installing it; you typically don't need to set any environment
variables (except maybe PATH) for the test-bin scripts to use
the build properly.
Trivial note: This is a cleaned up resend of part of a hodgepodge
cvsserver patch series that I sent last January.
---------
By the way, has anyone considered the possibility of splitting
up the large directory at the top of the git source tree? I
suspect that no one is interested, or it would have been done
already, but I thought I would mention it anyway. Perhaps split
off separate directories for libgit.a, builtins, other C-based
excutables, test support executables, scripts, output execdir, and
output intermediate (object) files for each of the other
directories. Or some subset of these.
--
Matthew Ogilvie [mmogilvi_git@miniinfo.net]
Matthew Ogilvie (4):
t2300: use documented technique to invoke git-sh-setup
t3409 t4107 t7406: use dashless commands
build dashless "test-bin" directory similar to installed bindir
run test suite without dashed git-commands in PATH
.gitignore | 1 +
INSTALL | 8 +++++-
Makefile | 46 +++++++++++++++++++++++++----------
t/README | 8 ++++++
t/t2300-cd-to-toplevel.sh | 2 +-
t/t3409-rebase-preserve-merges.sh | 6 ++--
t/t4107-apply-ignore-whitespace.sh | 20 ++++++++--------
t/t7406-submodule-update.sh | 4 +-
t/test-lib.sh | 33 ++++++++++++++++---------
test-bin-wrapper.sh | 13 ++++++++++
10 files changed, 99 insertions(+), 42 deletions(-)
create mode 100644 test-bin-wrapper.sh
^ permalink raw reply
* [RFC/PATCH] t7011: Mark fixed test as such
From: Michael J Gruber @ 2009-11-28 18:24 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy, Junio C Hamano
Test 16/17 had been fixed since its introduction in b4d1690 (Teach Git
to respect skip-worktree bit (reading part), 2009-08-20). So, mark it as
expect_success rather than expect_failure.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
I'm actually wondering about 17/17 as well.
If commit is called with a file name then shouldn't it simply commit the
current state of the file in the worktree, no matter what the index or
skip-worktree say? I therefore think 17/17 should be expect_success
and have no test_must_fail.
t/t7011-skip-worktree-reading.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t7011-skip-worktree-reading.sh b/t/t7011-skip-worktree-reading.sh
index e996928..8960dd9 100755
--- a/t/t7011-skip-worktree-reading.sh
+++ b/t/t7011-skip-worktree-reading.sh
@@ -148,7 +148,7 @@ test_expect_success 'git-rm succeeds on skip-worktree absent entries' '
git rm 1
'
-test_expect_failure 'commit on skip-worktree absent entries' '
+test_expect_success 'commit on skip-worktree absent entries' '
git reset &&
setup_absent &&
test_must_fail git commit -m null 1
--
1.6.6.rc0.274.g71380
^ permalink raw reply related
* non-US-ASCII file names (e.g. Hiragana) on Windows
From: Thomas Singer @ 2009-11-28 18:15 UTC (permalink / raw)
To: git
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?
--
Thanks in advance,
Tom
^ permalink raw reply
* Re: [PATCH 1/4] Makefile: fix http-push.o dependencies
From: Junio C Hamano @ 2009-11-28 18:05 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git
In-Reply-To: <20091128113115.GB10059@progeny.tock>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Since it is not in LIB_OBJS, http-push.o needs an explicit
> $(GIT_H) dependency.
I think you meant $(LIB_H).
$ make
$ touch cache.h
$ make http-push.o
does rebuild it with the current Makefile without this patch, because
it has this seemingly unrelated line (worse yet, this gives even more than
what are listed in LIB_H).
$(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(LIB_H) $(wildcard */*.h)
Puzzlingly messy.
^ permalink raw reply
* Re: Unable to checkout a particular SVN revision
From: Michael J Gruber @ 2009-11-28 15:03 UTC (permalink / raw)
To: Marc Liyanage; +Cc: git
In-Reply-To: <718EEBA2-FA4B-402D-B2FC-A8F14D79F6FF@entropy.ch>
Marc Liyanage venit, vidit, dixit 28.11.2009 03:05:
>
> I'm trying to clone a specific SVN revision with git-svn:
>
> git svn clone -r 12345 https://host/svn/foo/branches/bar xyz
>
> but it doesn't check out any files, I see just this:
>
> Initialized empty Git repository in /Users/liyanage/Desktop/xyz/.git
>
> If I try the same thing with SVN like this:
>
> svn co -r 12345 https://host/svn/foo/branches/bar xyz
>
> then I get what I expect, it checks out all the files and "svn info" gives me this revision.
>
>
> I think it's because this particular revision wasn't committed on this branch, i.e. it doesn't show up in "svn log". If I try a revision that is listed in the log, it works as expected.
>
>
> Is there a way to make this work?
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.
"svn co" checks out a specific revision.
You cannot "clone" a revision.
If all you want is a git repository with no history, but with the files
of a specific svn revision, you can
svn co -r 12345 https://host/svn/foo/branches/bar xyz
cd xyz
git init
find . -name .svn -print0 | xargs -0 rm -Rf
git add .
git commit -m "r12345 of branch bar"
Michael
^ permalink raw reply
* Re: [PATCH v3] Give the hunk comment its own color
From: Bert Wesarg @ 2009-11-28 12:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vhbsfi4bz.fsf@alter.siamese.dyndns.org>
On Sat, Nov 28, 2009 at 06:52, Junio C Hamano <gitster@pobox.com> wrote:
> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>
>> diff.c | 64 +++++++++++++++++++++++++++++++++++++++++++--
>> ...
>> @@ -344,6 +347,63 @@ static void emit_add_line(const char *reset,
>> }
>> }
>>
>> +static void emit_hunk_line(struct emit_callback *ecbdata,
>> + const char *line, int len)
>> +{
>> + const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);
>> + const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);
>> + const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);
>> + const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
>> + const char *orig_line = line;
>> + int orig_len = len;
>> + const char *frag_start;
>> + int frag_len;
>> + const char *part_end = NULL;
>> + int part_len = 0;
>> +
>> + /* determine length of @ */
>> + while (part_len < len && line[part_len] == '@')
>> + part_len++;
>> +
>> + /* find end of frag, (Ie. find second @@) */
>> + part_end = memmem(line + part_len, len - part_len,
>> + line, part_len);
>
> This is not incorrect per-se, but probably is overkill; this codepath only
> deals with two-way diff and we know we are looking at "@@ -..., +... @@"
> at this point.
>
> part_end = memmem(line + 2, len - 2, "@@", 2);
>
> would be sufficient.
Thats right, I made it generic by purpose.
>
>> + if (!part_end)
>> + return emit_line(ecbdata->file, frag, reset, line, len);
>> + /* calculate total length of frag */
>> + part_len = (part_end + part_len) - line;
>> +
>> + /* remember frag part, we emit only if we find a space separator */
>> + frag_start = line;
>> + frag_len = part_len;
>> +
>> + /* consume hunk header */
>> + len -= part_len;
>> + line += part_len;
>> +
>> + /*
>> + * for empty reminder or empty space sequence (exclusive any newlines
>> + * or carriage returns) emit complete original line as FRAGINFO
>> + */
>> + if (!len || !(part_len = strspn(line, " \t")))
>
> Slightly worrisome is what guarantees this strspn() won't step outside
> len.
Thats a valid concern and should be addressed.
>
> I would probably write the function like this instead.
>
> -- >8 --
>
> static void emit_hunk_header(struct emit_callback *ecbdata,
> const char *line, int len)
> {
> const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);
> const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);
> const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);
> const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
> static const char atat[2] = { '@', '@' };
> const char *cp, *ep;
>
> /*
> * As a hunk header must begin with "@@ -<old>, +<new> @@",
> * it always is at least 10 bytes long.
> */
> if (len < 10 ||
> memcmp(line, atat, 2) ||
> !(ep = memmem(line + 2, len - 2, atat, 2))) {
> emit_line(ecbdata->file, plain, reset, line, len);
> return;
> }
> ep += 2; /* skip over the second @@ */
>
> /* The hunk header in fraginfo color */
> emit_line(ecbdata->file, frag, reset, line, ep - line);
>
> /* blank before the func header */
> for (cp = ep; ep - line < len; ep++)
> if (*ep != ' ' && *ep != 't')
> break;
> if (ep != cp)
> emit_line(ecbdata->file, plain, reset, cp, ep - cp);
>
> if (ep < line + len)
> emit_line(ecbdata->file, func, reset, ep, line + len - ep);
> }
Please check that its really an *ep != '\t'. Its wrong in this mail, I
see only an *ep != 't'. Otherwise:
Acked-by: Bert.Wesarg@googlemail.com
>
>
^ permalink raw reply
* [PATCH v2 3/3] format-patch: add test for dashdash
From: Felipe Contreras @ 2009-11-28 11:40 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
In-Reply-To: <1259408429-5685-1-git-send-email-felipe.contreras@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
t/t4014-format-patch.sh | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 7f267f9..d5b002d 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -536,6 +536,10 @@ test_expect_success 'format-patch --signoff' '
grep "^Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
'
+test_expect_success 'format-patch -- <path>' '
+ git format-patch master..side -- file
+'
+
echo "fatal: --name-only does not make sense" > expect.name-only
echo "fatal: --name-status does not make sense" > expect.name-status
echo "fatal: --check does not make sense" > expect.check
--
1.6.6.rc0.59.g5117f7.dirty
^ permalink raw reply related
* [PATCH v2 2/3] format-patch: make full-diff enabled by default
From: Felipe Contreras @ 2009-11-28 11:40 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
In-Reply-To: <1259408429-5685-1-git-send-email-felipe.contreras@gmail.com>
It doesn't make much sense to generate partial patches (with some paths
omitted).
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin-log.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index 1766349..1e06859 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -960,6 +960,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
rev.diff = 1;
rev.combine_merges = 0;
rev.ignore_merges = 1;
+ rev.full_diff = 1;
DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
rev.subject_prefix = fmt_patch_subject_prefix;
--
1.6.6.rc0.59.g5117f7.dirty
^ permalink raw reply related
* [PATCH v2 0/3] format-patch: improve pathspec usage
From: Felipe Contreras @ 2009-11-28 11:40 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
Even though it's not documented, users still can make use of pathspecs in 'git
format-patch'. This patch series improves the way it works.
First, parse_options must not eat the '--', then, full-diff is the only way
pathspecs make sense.
This is v2, after comments from Junio. Still pending are the improvements in
documentation to explain how pathspecs make sense.
Felipe Contreras (3):
format-patch: fix dashdash usage
format-patch: make full-diff enabled by default
format-patch: add test for dashdash
builtin-log.c | 4 +++-
t/t4014-format-patch.sh | 4 ++++
2 files changed, 7 insertions(+), 1 deletions(-)
^ permalink raw reply
* [PATCH v2 1/3] format-patch: fix dashdash usage
From: Felipe Contreras @ 2009-11-28 11:40 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
In-Reply-To: <1259408429-5685-1-git-send-email-felipe.contreras@gmail.com>
$ git format-patch <committish> -- <path-not-in-working-dir>
Doesn't work otherwise. The current code would complain that the path is
not in the working tree and that "--" must be specified, even if the
user _did_ specify it. This happens because "--" is removed from the
arguments, so we need to pass PARSE_OPT_KEEP_DASHDASH to parse_options
to avoid that.
Comments by Junio C Hamano.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin-log.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index 33fa6ea..1766349 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -976,7 +976,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
*/
argc = parse_options(argc, argv, prefix, builtin_format_patch_options,
builtin_format_patch_usage,
- PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);
+ PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
+ PARSE_OPT_KEEP_DASHDASH);
if (do_signoff) {
const char *committer;
--
1.6.6.rc0.59.g5117f7.dirty
^ permalink raw reply related
* Re: [PATCH/RFC 2/2 v3] Makefile: lazily compute header dependencies
From: Jonathan Nieder @ 2009-11-28 11:49 UTC (permalink / raw)
To: Andreas Schwab
Cc: Johannes Schindelin, Sverre Rabbelier, Johannes Sixt,
Junio C Hamano, Jeff King, Git Mailing List
In-Reply-To: <m2bpint2yk.fsf@igel.home>
Andreas Schwab wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> GIT-CFLAGS: .FORCE-GIT-CFLAGS
>> + mkdir -p deps block-sha1/deps ppc/deps compat/deps \
>> + compat/regex/deps compat/nedmalloc/deps compat/fnmatch/deps \
>> + xdiff/deps
>
> IMHO the list of directories should be factored out in a variable for
> easier maintenance.
Good idea, thanks.
Perhaps the the value for such a variable could be computed at build time.
e.g.
depdirs := $(addsuffix /deps,$(sort $(dir $(OBJECTS))))
>> @@ -1873,8 +1898,10 @@ distclean: clean
>> $(RM) configure
>>
>> clean:
>> - $(RM) *.o block-sha1/*.o arm/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
>> + $(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
>> $(LIB_FILE) $(XDIFF_LIB)
>> + $(RM) -r deps block-sha1/deps ppc/deps compat/deps \
>> + compat/*/deps xdiff/deps
^ permalink raw reply
* [PATCH 4/4] Makefile: do not clean arm directory
From: Jonathan Nieder @ 2009-11-28 11:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20091128112546.GA10059@progeny.tock>
The ARM SHA-1 implementation was removed by commit 30ae47b
(remove ARM and Mozilla SHA1 implementations, 2009-08-17). Prune
its directory from the list of object files to delete in 'make
clean'.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
One could argue that this should be left in, to allow 'make
clean' to clean up after the old version in an upgrade. But that
way lies long rules for clean that never get tested for their
intended purpose.
Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index cd210e3..47e1412 100644
--- a/Makefile
+++ b/Makefile
@@ -1833,7 +1833,7 @@ distclean: clean
$(RM) configure
clean:
- $(RM) *.o block-sha1/*.o arm/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
+ $(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
$(LIB_FILE) $(XDIFF_LIB)
$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
$(RM) $(TEST_PROGRAMS)
--
1.6.5.3
^ permalink raw reply related
* [PATCH 3/4] Makefile: fix .s pattern rule dependencies
From: Jonathan Nieder @ 2009-11-28 11:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20091128112546.GA10059@progeny.tock>
'make git.s' fails to regenerate an assembler listing if git.c
has not changed but a header it includes has. The %.s: %.c
pattern rule is meant to be invoked by hand, so it would be
better to make it always run.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
This adds yet another phony .FORCE-foo target. Wouldn’t it be simpler
to use a single target called .FORCE, or is there something I am
missing that that would break?
Makefile | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index bb3879e..cd210e3 100644
--- a/Makefile
+++ b/Makefile
@@ -1524,7 +1524,7 @@ git.o git.spec \
%.o: %.c GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
-%.s: %.c GIT-CFLAGS
+%.s: %.c GIT-CFLAGS .FORCE-LISTING
$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
%.o: %.S GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
@@ -1859,7 +1859,7 @@ endif
.PHONY: all install clean strip
.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
.PHONY: .FORCE-GIT-VERSION-FILE TAGS tags cscope .FORCE-GIT-CFLAGS
-.PHONY: .FORCE-GIT-BUILD-OPTIONS
+.PHONY: .FORCE-GIT-BUILD-OPTIONS .FORCE-LISTING
### Check documentation
#
--
1.6.5.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox