* Re: [PATCH 1/9] close another possibility for propagating pack corruption
From: Nicolas Pitre @ 2008-10-31 15:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vod11884o.fsf@gitster.siamese.dyndns.org>
On Fri, 31 Oct 2008, Junio C Hamano wrote:
> Nicolas Pitre <nico@cam.org> writes:
>
> > Abstract
> > --------
> >
> > With index v2 we have a per object CRC to allow quick and safe reuse of
> > pack data when repacking. this, however, doesn't currently prevent a
>
> s/this/This/;
Thanks for proofreading. ;-)
> Very nicely done. I've never seen a commit message that needs its own
> Abstract ;-)
Well, maybe I could run for the best commit message award! :-)
> > diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> > index 59c30d1..0366277 100644
> > --- a/builtin-pack-objects.c
> > +++ b/builtin-pack-objects.c
> > @@ -1689,6 +1689,8 @@ static int add_ref_tag(const char *path, const unsigned char *sha1, int flag, vo
> > return 0;
> > }
> >
> > +extern int do_check_packed_object_crc;
> > +
>
> This ought to belong to cache.h or some other header file. Perhaps you
> did this to avoid unnecessary recompilation (we've discussed this at
> GitTogether)?
No, I just felt this wasn't a really public thing to expose at large.
But I don't feel really strongly about that.
Revised patch follows.
--------
From: Nicolas Pitre <nico@cam.org>
Date: Tue, 28 Oct 2008 20:58:42 -0400
Subject: [PATCH] close another possibility for propagating pack corruption
Abstract
--------
With index v2 we have a per object CRC to allow quick and safe reuse of
pack data when repacking. This, however, doesn't currently prevent a
stealth corruption from being propagated into a new pack when _not_
reusing pack data as demonstrated by the modification to t5302 included
here.
The Context
-----------
The Git database is all checksummed with SHA1 hashes. Any kind of
corruption can be confirmed by verifying this per object hash against
corresponding data. However this can be costly to perform systematically
and therefore this check is often not performed at run time when
accessing the object database.
First, the loose object format is entirely compressed with zlib which
already provide a CRC verification of its own when inflating data. Any
disk corruption would be caught already in this case.
Then, packed objects are also compressed with zlib but only for their
actual payload. The object headers and delta base references are not
deflated for obvious performance reasons, however this leave them
vulnerable to potentially undetected disk corruptions. Object types
are often validated against the expected type when they're requested,
and deflated size must always match the size recorded in the object header,
so those cases are pretty much covered as well.
Where corruptions could go unnoticed is in the delta base reference.
Of course, in the OBJ_REF_DELTA case, the odds for a SHA1 reference to
get corrupted so it actually matches the SHA1 of another object with the
same size (the delta header stores the expected size of the base object
to apply against) are virtually zero. In the OBJ_OFS_DELTA case, the
reference is a pack offset which would have to match the start boundary
of a different base object but still with the same size, and although this
is relatively much more "probable" than in the OBJ_REF_DELTA case, the
probability is also about zero in absolute terms. Still, the possibility
exists as demonstrated in t5302 and is certainly greater than a SHA1
collision, especially in the OBJ_OFS_DELTA case which is now the default
when repacking.
Again, repacking by reusing existing pack data is OK since the per object
CRC provided by index v2 guards against any such corruptions. What t5302
failed to test is a full repack in such case.
The Solution
------------
As unlikely as this kind of stealth corruption can be in practice, it
certainly isn't acceptable to propagate it into a freshly created pack.
But, because this is so unlikely, we don't want to pay the run time cost
associated with extra validation checks all the time either. Furthermore,
consequences of such corruption in anything but repacking should be rather
visible, and even if it could be quite unpleasant, it still has far less
severe consequences than actively creating bad packs.
So the best compromize is to check packed object CRC when unpacking
objects, and only during the compression/writing phase of a repack, and
only when not streaming the result. The cost of this is minimal (less
than 1% CPU time), and visible only with a full repack.
Someone with a stats background could provide an objective evaluation of
this, but I suspect that it's bad RAM that has more potential for data
corruptions at this point, even in those cases where this extra check
is not performed. Still, it is best to prevent a known hole for
corruption when recreating object data into a new pack.
What about the streamed pack case? Well, any client receiving a pack
must always consider that pack as untrusty and perform full validation
anyway, hence no such stealth corruption could be propagated to remote
repositoryes already. It is therefore worthless doing local validation
in that case.
Signed-off-by: Nicolas Pitre <nico@cam.org>
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 59c30d1..026b7ea 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1697,6 +1697,16 @@ static void prepare_pack(int window, int depth)
get_object_details();
+ /*
+ * If we're locally repacking then we need to be doubly careful
+ * from now on in order to make sure no stealth corruption gets
+ * propagated to the new pack. Clients receiving streamed packs
+ * should validate everything they get anyway so no need to incur
+ * the additional cost here in that case.
+ */
+ if (!pack_to_stdout)
+ do_check_packed_object_crc = 1;
+
if (!nr_objects || !window || !depth)
return;
diff --git a/cache.h b/cache.h
index a3c77f0..0dc13cc 100644
--- a/cache.h
+++ b/cache.h
@@ -565,6 +565,9 @@ extern int force_object_loose(const unsigned char *sha1, time_t mtime);
/* just like read_sha1_file(), but non fatal in presence of bad objects */
extern void *read_object(const unsigned char *sha1, enum object_type *type, unsigned long *size);
+/* global flag to enable extra checks when accessing packed objects */
+extern int do_check_packed_object_crc;
+
extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned long size, const char *type);
extern int move_temp_to_file(const char *tmpfile, const char *filename);
diff --git a/sha1_file.c b/sha1_file.c
index ab2b520..88d9cf3 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1694,6 +1694,8 @@ static void *unpack_delta_entry(struct packed_git *p,
return result;
}
+int do_check_packed_object_crc;
+
void *unpack_entry(struct packed_git *p, off_t obj_offset,
enum object_type *type, unsigned long *sizep)
{
@@ -1701,6 +1703,19 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,
off_t curpos = obj_offset;
void *data;
+ if (do_check_packed_object_crc && p->index_version > 1) {
+ struct revindex_entry *revidx = find_pack_revindex(p, obj_offset);
+ unsigned long len = revidx[1].offset - obj_offset;
+ if (check_pack_crc(p, &w_curs, obj_offset, len, revidx->nr)) {
+ const unsigned char *sha1 =
+ nth_packed_object_sha1(p, revidx->nr);
+ error("bad packed object CRC for %s",
+ sha1_to_hex(sha1));
+ mark_bad_packed_object(p, sha1);
+ return NULL;
+ }
+ }
+
*type = unpack_object_header(p, &w_curs, &curpos, sizep);
switch (*type) {
case OBJ_OFS_DELTA:
diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
index b0b0fda..884e242 100755
--- a/t/t5302-pack-index.sh
+++ b/t/t5302-pack-index.sh
@@ -196,7 +196,8 @@ test_expect_success \
test_expect_success \
'[index v2] 5) pack-objects refuses to reuse corrupted data' \
- 'test_must_fail git pack-objects test-5 <obj-list'
+ 'test_must_fail git pack-objects test-5 <obj-list &&
+ test_must_fail git pack-objects --no-reuse-object test-6 <obj-list'
test_expect_success \
'[index v2] 6) verify-pack detects CRC mismatch' \
^ permalink raw reply related
* Re: [JGIT PATCH 0/3] Improved object validation
From: Shawn O. Pearce @ 2008-10-31 14:55 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <200810310101.12241.robin.rosenberg@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> torsdagen den 30 oktober 2008 18.46.22 skrev Shawn O. Pearce:
> > This is mostly a resend as I haven't heard anything on the series.
> > One new patch at the end, to handle '.' and '..' cases.
> Ack, pushed now. I never got to the push action, because I wanted
> to do some testing on the bundlewriter (which you did not resubmit) and
> the I went to a warmer place for a few days.
>
> These patches and the bundlewrite + a small unit test for it now pushed.
Thanks for putting together the bundle writer test. I had to fix
it a bit to avoid the /home/me path it had hardcoded in there.
This patch was applied on top of master and pushed out.
I meant to work on a test for BundleWriter myself, but I got too busy
with other stuff and totally forgot about it. I wasn't pushing to
get the BundleWriter included as it was non-critical to my current
needs, but I had it written so I posted it in case someone else
cared about it.
--8<--
[PATCH] Fix BundleWriter unit test to work for people who are not 'me'
My system lacks a /home/me as I do not call myself me. My login is
usually based upon my email address, and nobody else on my system is
'me'. So we cannot use hardcoded paths like /home/me in a test case.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../spearce/jgit/transport/BundleWriterTest.java | 68 ++++++++++----------
1 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/transport/BundleWriterTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/transport/BundleWriterTest.java
index 4e33108..3cfb8b1 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/transport/BundleWriterTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/transport/BundleWriterTest.java
@@ -37,11 +37,13 @@
package org.spearce.jgit.transport;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
import java.io.FileNotFoundException;
-import java.io.FileOutputStream;
import java.io.IOException;
import java.net.URISyntaxException;
import java.util.Collections;
+import java.util.Set;
import org.spearce.jgit.errors.MissingBundlePrerequisiteException;
import org.spearce.jgit.errors.NotSupportedException;
@@ -58,15 +60,14 @@
public void testWrite0() throws Exception {
// Create a tiny bundle, (well one of) the first commits only
- URIish bundleURI = new URIish("file:///home/me/tmp/foo");
- makeBundle("refs/heads/firstcommit",
- "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1", bundleURI, null);
+ final byte[] bundle = makeBundle("refs/heads/firstcommit",
+ "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1", null);
// Then we clone a new repo from that bundle and do a simple test. This
// makes sure
// we could read the bundle we created.
Repository newRepo = createNewEmptyRepo();
- FetchResult fetchResult = fetchFromBundle(newRepo, bundleURI);
+ FetchResult fetchResult = fetchFromBundle(newRepo, bundle);
Ref advertisedRef = fetchResult
.getAdvertisedRef("refs/heads/firstcommit");
@@ -80,19 +81,20 @@ assertEquals("42e4e7c5e507e113ebbb7801b16b52cf867b7ce1", newRepo
/**
* Incremental bundle test
- *
+ *
* @throws Exception
*/
public void testWrite1() throws Exception {
+ byte[] bundle;
+
// Create a small bundle, an early commit
- URIish bundleURI = new URIish("file:///home/me/tmp/foo");
- makeBundle("refs/heads/aa", db.resolve("a").name(), bundleURI, null);
+ bundle = makeBundle("refs/heads/aa", db.resolve("a").name(), null);
// Then we clone a new repo from that bundle and do a simple test. This
// makes sure
// we could read the bundle we created.
Repository newRepo = createNewEmptyRepo();
- FetchResult fetchResult = fetchFromBundle(newRepo, bundleURI);
+ FetchResult fetchResult = fetchFromBundle(newRepo, bundle);
Ref advertisedRef = fetchResult.getAdvertisedRef("refs/heads/aa");
assertEquals(db.resolve("a").name(), advertisedRef.getObjectId().name());
@@ -101,9 +103,9 @@ assertEquals(db.resolve("a").name(), newRepo.resolve("refs/heads/aa")
assertNull(newRepo.resolve("refs/heads/a"));
// Next an incremental bundle
- makeBundle("refs/heads/cc", db.resolve("c").name(), bundleURI,
+ bundle = makeBundle("refs/heads/cc", db.resolve("c").name(),
new RevWalk(db).parseCommit(db.resolve("a").toObjectId()));
- fetchResult = fetchFromBundle(newRepo, bundleURI);
+ fetchResult = fetchFromBundle(newRepo, bundle);
advertisedRef = fetchResult.getAdvertisedRef("refs/heads/cc");
assertEquals(db.resolve("c").name(), advertisedRef.getObjectId().name());
assertEquals(db.resolve("c").name(), newRepo.resolve("refs/heads/cc")
@@ -114,7 +116,7 @@ assertEquals(db.resolve("c").name(), newRepo.resolve("refs/heads/cc")
try {
// Check that we actually needed the first bundle
Repository newRepo2 = createNewEmptyRepo();
- fetchResult = fetchFromBundle(newRepo2, bundleURI);
+ fetchResult = fetchFromBundle(newRepo2, bundle);
fail("We should not be able to fetch from bundle with prerequisistes that are not fulfilled");
} catch (MissingBundlePrerequisiteException e) {
assertTrue(e.getMessage()
@@ -122,31 +124,29 @@ assertTrue(e.getMessage()
}
}
- private FetchResult fetchFromBundle(Repository newRepo, URIish uriish)
- throws URISyntaxException, NotSupportedException,
- TransportException {
- RemoteConfig remoteConfig = new RemoteConfig(newRepo.getConfig(),
- "origin");
- remoteConfig.addURI(uriish);
- RefSpec theRefSpec = new RefSpec("refs/heads/*:refs/heads/*");
- remoteConfig.addFetchRefSpec(theRefSpec);
- Transport transport = Transport.open(newRepo, remoteConfig);
- FetchResult fetch = transport.fetch(new NullProgressMonitor(),
- Collections.singleton(theRefSpec));
- return fetch;
+ private FetchResult fetchFromBundle(final Repository newRepo,
+ final byte[] bundle) throws URISyntaxException,
+ NotSupportedException, TransportException {
+ final URIish uri = new URIish("in-memory://");
+ final ByteArrayInputStream in = new ByteArrayInputStream(bundle);
+ final RefSpec rs = new RefSpec("refs/heads/*:refs/heads/*");
+ final Set<RefSpec> refs = Collections.singleton(rs);
+ return new TransportBundleStream(newRepo, uri, in).fetch(
+ NullProgressMonitor.INSTANCE, refs);
}
- private void makeBundle(String name, String anObjectToInclude,
- URIish bundleName, RevCommit assume) throws FileNotFoundException,
- IOException {
- BundleWriter bundleWriter = new BundleWriter(db,
- new NullProgressMonitor());
- bundleWriter.include(name, ObjectId.fromString(anObjectToInclude));
+ private byte[] makeBundle(final String name,
+ final String anObjectToInclude, final RevCommit assume)
+ throws FileNotFoundException, IOException {
+ final BundleWriter bw;
+
+ bw = new BundleWriter(db, NullProgressMonitor.INSTANCE);
+ bw.include(name, ObjectId.fromString(anObjectToInclude));
if (assume != null)
- bundleWriter.assume(assume);
- FileOutputStream os = new FileOutputStream(bundleName.getPath());
- bundleWriter.writeBundle(os);
- os.close();
+ bw.assume(assume);
+ final ByteArrayOutputStream out = new ByteArrayOutputStream();
+ bw.writeBundle(out);
+ return out.toByteArray();
}
}
--
1.6.0.3.756.gb776d
--
Shawn.
^ permalink raw reply related
* Re: jgit as a jira plugin
From: Shawn O. Pearce @ 2008-10-31 14:42 UTC (permalink / raw)
To: J. Longman; +Cc: git
In-Reply-To: <C9B1B0D7-6F99-48DE-8593-A13F1187ADE2@xiplink.com>
"J. Longman" <longman@xiplink.com> wrote:
> Right, I see now, I thought there was going to be more detail. Anyways,
> I'm wondering if there is some action required to ensure it reads from
> the database vs. the working directory? I'm assuming it is still looking
> at the workspace as this is what I'm seeing:
>
> ...RevisionIndexer] Latest indexed revision for repository=1 is :
> 29ed4398d047ba5e0d6fbad9ebbf98304b0fc503
> ...GitManagerImpl] Fetch...
> ...GitManagerImpl] From /Users/longman/workspace2/work/../masterRepo/
> ...GitManagerImpl] c209b0f..594e8ff master -> origin/master
> ...GitManagerImpl] scan for repo changes...
> repository.scanForRepoChanges();
> ...GitManagerImpl] scan for repo changes... complete
> ...RevisionIndexer] testing 29ed4398d047ba5e0d6fbad9ebbf98304b0fc503 at
> CORE-23 sdjskl
> ...RevisionIndexer] update latest as
> 29ed4398d047ba5e0d6fbad9ebbf98304b0fc503 at Wed Oct 29 18:49:54 EDT 2008
Oh.
Your RevisionIndexer must be looking at HEAD, which is the current
branch data. However the fetch process updated the remote tracking
refs, which are in a different namespace.
You should point your indexer at "origin/master", or use a bare
repository (one with no working directory) and fetch directly into
"refs/heads/*" instead of into "refs/remotes/origin/*", that way
the indexer can look at branch "master".
Fetch doesn't ever touch the working directory; it only updates
the database in the background and the refs/remotes/ namespace so
that other applications can see the data that was transferred and
choose how to process it.
In command line Git "git pull" uses "git fetch" to get the data
and then uses the received data to update the working directory.
But a lot of workflows also will just issue "git fetch" on their own
to get the data, then examine it with "gitk --all", or do nothing
at all because they are mirroring the data, or have to close their
laptop and catch a bus, etc...
If all you are doing is scanning the revision history and the
files via JGit you just have to run a Transport.fetch() call and
then look at the refs that were updated during that call. See the
FetchResult object you get returned for the list; it is what the
pgm.Fetch class uses to display the output cited above.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Avoid using non-portable `echo -n` in tests.
From: Shawn O. Pearce @ 2008-10-31 14:32 UTC (permalink / raw)
To: Brian Gernhardt; +Cc: Git List
In-Reply-To: <1225429753-70109-1-git-send-email-benji@silverinsanity.com>
Brian Gernhardt <benji@silverinsanity.com> wrote:
> Not all /bin/sh have a builtin echo that recognizes -n. Using printf
> is far more portable.
> diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
> index c1850d2..f6a2dbd 100755
> --- a/t/t9400-git-cvsserver-server.sh
> +++ b/t/t9400-git-cvsserver-server.sh
> @@ -424,7 +424,7 @@ cd "$WORKDIR"
> test_expect_success 'cvs update (-p)' '
> touch really-empty &&
> echo Line 1 > no-lf &&
> - echo -n Line 2 >> no-lf &&
> + printf Line 2 >> no-lf &&
That needs to be:
printf 'Line 2'
to have the same result. Fortunately I don't think it matters
in this test, but it does read odd.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Introduce receive.denyDeletes
From: Shawn O. Pearce @ 2008-10-31 14:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jan Krrrger, git
In-Reply-To: <7v63n99omx.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Jan Krüger" <jk@jk.gs> writes:
>
> > Can I then delete the branch afterwards without lots of juggling (in
> > case the test fails due to a random other reason that the branch
> > accidentally getting deleted by receive-pack)? I'd expect I'd have to
> > save the exit code to a temporary variable and that's just as ugly.
If you want to delete the branch after the test is done, do it
outside of the test_expect_success's 3rd argument. Then it will
run the branch deletion whether or not the test was successful.
> Although I agree that your attempt to allow the test continue even when
> this test fails is a very good practice, I personally do not find the
> alternative you mention ugly at all. I actually find that "return 1"
> uglier because it feels like it knows too much about how
> test_expect_success is implemented.
Yea, I also found the "return 1" to be horribly difficult to read, and
knowing far too much about the test suite.
--
Shawn.
^ permalink raw reply
* Re: Are binary xdeltas only used if you use git-gc?
From: Thanassis Tsiodras @ 2008-10-31 14:22 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqy705rl5u.fsf@bauges.imag.fr>
Actually, I am not so worried about disk size - I am far more worried
about how long it takes to git-push over my PSTN modem connection.
I'll try Jakub's suggestion (git-gc on both my machine and the remote
machine hosting the repos) and report back.
On Fri, Oct 31, 2008 at 2:42 PM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> If you're worried about repository size and you have a permanently
> running machine, a good idea is to run git gc in a cron job, so that
> you work fast in daytime, and your computer optimizes hard at night
> time ;-) (I have gic gc + git fsck in a cron job, so I'll also know if
> a repository gets corrupted).
>
> --
> Matthieu
>
--
What I gave, I have; what I spent, I had; what I kept, I lost. -Old Epitaph
^ permalink raw reply
* Re: [PATCH] Use find instead of perl in t5000 to get file modification time
From: Alex Riesen @ 2008-10-31 13:45 UTC (permalink / raw)
To: Peter Harris
Cc: Sam Vilain, Git Mailing List, Junio C Hamano, Jeff King,
René Scharfe
In-Reply-To: <eaa105840810310559l29addd80i7a22c21e37231bb1@mail.gmail.com>
2008/10/31 Peter Harris <git@peter.is-a-geek.org>:
> On Fri, Oct 31, 2008 at 3:00 AM, Alex Riesen wrote:
>> Sam Vilain, Thu, Oct 30, 2008 06:29:14 +0100:
>>> On Wed, 2008-10-29 at 11:38 +0100, Alex Riesen wrote:
>>> > I could not find what exactly does the ActiveState's Perl use for its stat
>>> > implementation (and honestly, have no motivation to look harder).
>>> > It seems to honor TZ, but the produced time does not seem to be either
>>> > local or GMT.
>>>
>>> See, the difference is that the perl is portable and your patch isn't.
>>
>> ActiveState Perl on Windows is portable? To another windows, maybe.
>
> Quite aside from missing the point (which is that Vanilla Perl runs
> everywhere, including Windows[1]), this is also factually incorrect.
Ok, it is (almost: "Vanilla Perl is experimental and is not intended for
production purposes". Well, I need it exactly for production purposes!)
> A quick check of the ActiveState website would reveal ActivePerl
> downloads for AIX, Linux (x86 and x86-64), Solaris (x86, SPARC, and
> SPARC64), MacOSX (x86 and PPC), and source code, in addition to
> Windows.
what's the point of that, I wonder... Just to admit they broke the original
source beyond all repair on one platform (windows) so now they
have provide their poor customers with the same broken version
on all the other platforms, because they complained about incompatibilities.
So Perl is portable. ActiveState Perl may look portable. They just don't
seem to be compatible enough. Even then, the Git's test suite and some
scripts (I have no use for git-svn on Windows) do very good job to be usable
even if the portable dependency replaced with something else. I'm trying to
keep it at that because I personally have no choice.
^ permalink raw reply
* Re: [PATCH] Use find instead of perl in t5000 to get file modification time
From: Peter Harris @ 2008-10-31 12:59 UTC (permalink / raw)
To: Alex Riesen
Cc: Sam Vilain, Git Mailing List, Junio C Hamano, Jeff King,
René Scharfe
In-Reply-To: <20081031070003.GA4458@blimp.localdomain>
On Fri, Oct 31, 2008 at 3:00 AM, Alex Riesen wrote:
> Sam Vilain, Thu, Oct 30, 2008 06:29:14 +0100:
>> On Wed, 2008-10-29 at 11:38 +0100, Alex Riesen wrote:
>> > I could not find what exactly does the ActiveState's Perl use for its stat
>> > implementation (and honestly, have no motivation to look harder).
>> > It seems to honor TZ, but the produced time does not seem to be either
>> > local or GMT.
>>
>> See, the difference is that the perl is portable and your patch isn't.
>
> ActiveState Perl on Windows is portable? To another windows, maybe.
Quite aside from missing the point (which is that Vanilla Perl runs
everywhere, including Windows[1]), this is also factually incorrect. A
quick check of the ActiveState website would reveal ActivePerl
downloads for AIX, Linux (x86 and x86-64), Solaris (x86, SPARC, and
SPARC64), MacOSX (x86 and PPC), and source code, in addition to
Windows.
Looks pretty portable to me.
Peter Harris
[1] http://vanillaperl.com/
^ permalink raw reply
* Re: Are binary xdeltas only used if you use git-gc?
From: Matthieu Moy @ 2008-10-31 12:42 UTC (permalink / raw)
To: Thanassis Tsiodras; +Cc: git
In-Reply-To: <f1d2d9ca0810310243r669840bbj2c5ee7183e0caaed@mail.gmail.com>
"Thanassis Tsiodras" <ttsiodras@gmail.com> writes:
> If that is the case, I will create an alias to always git-gc after commits...
If you have a decent version of git, it already does "git gc --auto"
regularly. With --auto, git gc will do nothing if you don't have too
many unpacked objects, and will try to do the right thing otherwise
(incremental packs, see man git gc). The idea is that "git gc" is a
costly operation, and git prefers to waste a bit of disk space to make
most "commit" really fast, and to take time to optimize the repository
only when it grew too much.
If you're worried about repository size and you have a permanently
running machine, a good idea is to run git gc in a cron job, so that
you work fast in daytime, and your computer optimizes hard at night
time ;-) (I have gic gc + git fsck in a cron job, so I'll also know if
a repository gets corrupted).
--
Matthieu
^ permalink raw reply
* [PATCH 2/3] git send-email: do not ask questions when --compose is used.
From: Pierre Habouzit @ 2008-10-31 12:36 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <1225456609-694-2-git-send-email-madcoder@debian.org>
When --compose is used, we can grab the From/Subject/In-Reply-To from the
edited summary, let it be so and don't ask the user silly questions.
The summary templates gets quite revamped, and includes the list of
patches subjects that are going to be sent with this batch.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
git-send-email.perl | 174 ++++++++++++++++++++++++++++++---------------------
1 files changed, 102 insertions(+), 72 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 4ca571f..5c189a7 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -417,6 +417,105 @@ if (@files) {
usage();
}
+sub get_patch_subject($) {
+ my $fn = shift;
+ open (my $fh, '<', $fn);
+ while (my $line = <$fh>) {
+ next unless ($line =~ /^Subject: (.*)$/);
+ close $fh;
+ return "GIT: $1\n";
+ }
+ close $fh;
+ die "No subject line in $fn ?";
+}
+
+if ($compose) {
+ # Note that this does not need to be secure, but we will make a small
+ # effort to have it be unique
+ open(C,">",$compose_filename)
+ or die "Failed to open for writing $compose_filename: $!";
+
+
+ my $tpl_sender = $sender || $repoauthor || $repocommitter || '';
+ my $tpl_subject = $initial_subject || '';
+ my $tpl_reply_to = $initial_reply_to || '';
+
+ print C <<EOT;
+From $tpl_sender # This line is ignored.
+GIT: Lines beginning in "GIT: " will be removed.
+GIT: Consider including an overall diffstat or table of contents
+GIT: for the patch you are writing.
+From: $tpl_sender
+Subject: $tpl_subject
+In-Reply-To: $tpl_reply_to
+
+GIT: Please enter your email below this line.
+
+EOT
+ for my $f (@files) {
+ print C get_patch_subject($f);
+ }
+ close(C);
+
+ my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
+
+ if ($annotate) {
+ do_edit($compose_filename, @files);
+ } else {
+ do_edit($compose_filename);
+ }
+
+ open(C2,">",$compose_filename . ".final")
+ or die "Failed to open $compose_filename.final : " . $!;
+
+ open(C,"<",$compose_filename)
+ or die "Failed to open $compose_filename : " . $!;
+
+ my $need_8bit_cte = file_has_nonascii($compose_filename);
+ my $in_body = 0;
+ my $summary_empty = 1;
+ while(<C>) {
+ next if m/^GIT: /;
+ if ($in_body) {
+ } elsif (/^\n$/) {
+ $in_body = 1;
+ if ($need_8bit_cte) {
+ print C2 "MIME-Version: 1.0\n",
+ "Content-Type: text/plain; ",
+ "charset=utf-8\n",
+ "Content-Transfer-Encoding: 8bit\n";
+ }
+ } elsif (/^MIME-Version:/i) {
+ $need_8bit_cte = 0;
+ } elsif (/^Subject:\s*(.+)\s*$/i) {
+ $initial_subject = $1;
+ my $subject = $initial_subject;
+ $_ = "Subject: " .
+ ($subject =~ /[^[:ascii:]]/ ?
+ quote_rfc2047($subject) :
+ $subject) .
+ "\n";
+ } elsif (/^In-Reply-To:\s*(.+)\s*$/i) {
+ $initial_reply_to = $1;
+ next;
+ } elsif (/^From:\s*(.+)\s*$/i) {
+ $sender = $1;
+ next;
+ }
+ $summary_empty = 0;
+ print C2 $_;
+ }
+ close(C);
+ close(C2);
+
+ if ($summary_empty) {
+ print "Summary email is empty, skpping it\n";
+ $compose = -1;
+ }
+} elsif ($annotate) {
+ do_edit(@files);
+}
+
my $prompting = 0;
if (!defined $sender) {
$sender = $repoauthor || $repocommitter || '';
@@ -461,17 +560,6 @@ sub expand_aliases {
@initial_cc = expand_aliases(@initial_cc);
@bcclist = expand_aliases(@bcclist);
-if (!defined $initial_subject && $compose) {
- while (1) {
- $_ = $term->readline("What subject should the initial email start with? ", $initial_subject);
- last if defined $_;
- print "\n";
- }
-
- $initial_subject = $_;
- $prompting++;
-}
-
if ($thread && !defined $initial_reply_to && $prompting) {
while (1) {
$_= $term->readline("Message-ID to be used as In-Reply-To for the first email? ", $initial_reply_to);
@@ -498,64 +586,6 @@ if (!defined $smtp_server) {
}
if ($compose) {
- # Note that this does not need to be secure, but we will make a small
- # effort to have it be unique
- open(C,">",$compose_filename)
- or die "Failed to open for writing $compose_filename: $!";
- print C "From $sender # This line is ignored.\n";
- printf C "Subject: %s\n\n", $initial_subject;
- printf C <<EOT;
-GIT: Please enter your email below.
-GIT: Lines beginning in "GIT: " will be removed.
-GIT: Consider including an overall diffstat or table of contents
-GIT: for the patch you are writing.
-
-EOT
- close(C);
-
- my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
-
- if ($annotate) {
- do_edit($compose_filename, @files);
- } else {
- do_edit($compose_filename);
- }
-
- open(C2,">",$compose_filename . ".final")
- or die "Failed to open $compose_filename.final : " . $!;
-
- open(C,"<",$compose_filename)
- or die "Failed to open $compose_filename : " . $!;
-
- my $need_8bit_cte = file_has_nonascii($compose_filename);
- my $in_body = 0;
- while(<C>) {
- next if m/^GIT: /;
- if (!$in_body && /^\n$/) {
- $in_body = 1;
- if ($need_8bit_cte) {
- print C2 "MIME-Version: 1.0\n",
- "Content-Type: text/plain; ",
- "charset=utf-8\n",
- "Content-Transfer-Encoding: 8bit\n";
- }
- }
- if (!$in_body && /^MIME-Version:/i) {
- $need_8bit_cte = 0;
- }
- if (!$in_body && /^Subject: ?(.*)/i) {
- my $subject = $1;
- $_ = "Subject: " .
- ($subject =~ /[^[:ascii:]]/ ?
- quote_rfc2047($subject) :
- $subject) .
- "\n";
- }
- print C2 $_;
- }
- close(C);
- close(C2);
-
while (1) {
$_ = $term->readline("Send this email? (y|n) ");
last if defined $_;
@@ -567,9 +597,9 @@ EOT
exit(0);
}
- @files = ($compose_filename . ".final", @files);
-} elsif ($annotate) {
- do_edit(@files);
+ if ($compose > 0) {
+ @files = ($compose_filename . ".final", @files);
+ }
}
# Variables we set as part of the loop over files
--
1.6.0.3.763.g0275.dirty
^ permalink raw reply related
* Further enhancement proposal for git-send-email
From: Pierre Habouzit @ 2008-10-31 12:36 UTC (permalink / raw)
To: git
In-Reply-To: <1225450632-7230-1-git-send-email-madcoder@debian.org>
Here is a three patch series (again).
[PATCH 1/3] git send-email: make the message file name more specific.
-> quite independant, and should IMHO be taken.
[PATCH 2/3] git send-email: do not ask questions when --compose is used.
[PATCH 3/3] git send-email: turn --compose on when more than one patch.
Those two patches enhance git-send-email by making ask less questions
when --compose is used (as it can grab the subject, from and reply-to
from the buffer).
It also turns --compose on by default as soon as there is more than
one patch, as I believe than commenting a patch series is more often
done than not. It's is really trivial to "refuse" to comment the
series by just erasing the full buffer content, which should not
really be too anoying (or one can explicitely pass --no-compose for
the same result).
It's probable that those two changes may trigger some discussion
though, but I just used that to send this series, and I can tell with
this git-send-email is nearer what I would like it to be.
^ permalink raw reply
* [PATCH 1/3] git send-email: make the message file name more specific.
From: Pierre Habouzit @ 2008-10-31 12:36 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <1225456609-694-1-git-send-email-madcoder@debian.org>
This helps editors choosing their syntax hilighting properly.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
git-send-email.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 65c254d..4ca571f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -127,7 +127,7 @@ sub unique_email_list(@);
sub cleanup_compose_files();
# Constants (essentially)
-my $compose_filename = ".msg.$$";
+my $compose_filename = ".gitsendemail.msg.$$";
# Variables we fill in automatically, or via prompting:
my (@to,@cc,@initial_cc,@bcclist,@xh,
--
1.6.0.3.763.g0275.dirty
^ permalink raw reply related
* [PATCH 3/3] git send-email: turn --compose on when more than one patch.
From: Pierre Habouzit @ 2008-10-31 12:36 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <1225456609-694-3-git-send-email-madcoder@debian.org>
Automatically turn --compose on when there is more than one patch, and
that the output is a tty.
Do not print the list of files sent anymore in that case, as the list is
shown in the summary editor.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
git-send-email.perl | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 5c189a7..5cebb40 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -49,7 +49,7 @@ git send-email [options] <file | directory | rev-list >
--subject <str> * Email "Subject:"
--in-reply-to <str> * Email "In-Reply-To:"
--annotate * Review each patch that will be sent in an editor.
- --compose * Open an editor for introduction.
+ --[no-]compose * Open an editor for introduction.
Sending:
--envelope-sender <str> * Email envelope sender.
@@ -237,7 +237,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
"smtp-encryption=s" => \$smtp_encryption,
"identity=s" => \$identity,
"annotate" => \$annotate,
- "compose" => \$compose,
+ "compose!" => \$compose,
"quiet" => \$quiet,
"cc-cmd=s" => \$cc_cmd,
"suppress-from!" => \$suppress_from,
@@ -409,7 +409,11 @@ if ($validate) {
}
if (@files) {
- unless ($quiet) {
+ if (!defined($compose) && -t STDOUT) {
+ # turn $compose on if there is more than one file
+ $compose = $#files;
+ }
+ unless ($quiet || $compose) {
print $_,"\n" for (@files);
}
} else {
--
1.6.0.3.763.g0275.dirty
^ permalink raw reply related
* Re: why not TortoiseGit
From: Andreas Ericsson @ 2008-10-31 12:35 UTC (permalink / raw)
To: Ian Hilt; +Cc: Li Frank-B20596, git
In-Reply-To: <20081031121913.GE18221@sys-0.hiltweb.site>
Ian Hilt wrote:
> On Fri, Oct 31, 2008 at 09:44:45AM +0800, Li Frank-B20596 wrote:
>> There are TortoiseCVS, TortoiseSVN, TortoiseBzr, TortoiseHg
>> Why not ToroiseGit
>
> This is what Johannes Schindelin had to say,
>
> <http://code.google.com/p/msysgit/wiki/GitCheetah>
Noone's written TortoiseGit yet. I have no idea why, and I have
no reason to write it myself. If GitCheetah isn't working well,
I'm sure patches are welcome.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: why not TortoiseGit
From: Ian Hilt @ 2008-10-31 12:19 UTC (permalink / raw)
To: Li Frank-B20596; +Cc: git
In-Reply-To: <7FD1F85C96D70C4A89DA1DF7667EAE96125890@zch01exm23.fsl.freescale.net>
On Fri, Oct 31, 2008 at 09:44:45AM +0800, Li Frank-B20596 wrote:
> There are TortoiseCVS, TortoiseSVN, TortoiseBzr, TortoiseHg
> Why not ToroiseGit
This is what Johannes Schindelin had to say,
<http://code.google.com/p/msysgit/wiki/GitCheetah>
^ permalink raw reply
* Re: [PATCH] Documentation: add a planning document for the next CLI revamp
From: Kyle Moffett @ 2008-10-31 11:38 UTC (permalink / raw)
To: Stefan Karpinski; +Cc: Sam Vilain, git
In-Reply-To: <d4bc1a2a0810300355q42b35a35p2ba0e778691a0ab6@mail.gmail.com>
On Thu, Oct 30, 2008 at 6:55 AM, Stefan Karpinski
<stefan.karpinski@gmail.com> wrote:
> On Wed, Oct 29, 2008 at 8:48 PM, Sam Vilain <sam@vilain.net> wrote:
>> + * 'git export' command that does what
>> + 'git archive --format=tar --prefix=dir | tar x' does now
>
> It would be nice if the "git export" command could "checkout" a
> non-repo copy of a remote repo at a specific version. This would be as
> simple as calling archive on the remote size and then unarchiving it
> locally. But would of course take care of all the plumbing.
I'm not sure whether the "git archive | tar" or the following is more efficient:
export GIT_INDEX_FILE="$(mktemp .git/export-index.XXXXXX)"
git read-tree -i "$1"
git checkout-index -f -a --prefix="$2/"
Cheers,
Kyle Moffett
^ permalink raw reply
* Re: Are binary xdeltas only used if you use git-gc?
From: Thanassis Tsiodras @ 2008-10-31 11:28 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <m37i7pggnk.fsf@localhost.localdomain>
On Fri, Oct 31, 2008 at 1:15 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> I think you can use clean / smudge filter in gitattributes for that.
Thanks, I didn't know about that. Will look into it
> Git does deltification _only_ in packfiles. But when you push via SSH
> git would generate a pack file with commits the other side doesn't
> have, and those packs are thin packs, so they also have deltas... but
> the remote side then adds bases to those thin packs making them
> standalone: you would have to git-gc on remote.
So I have to git-gc on my side (after the commits), git-gc on the remote,
and then git-push?
What I gave, I have; what I spent, I had; what I kept, I lost. -Old Epitaph
^ permalink raw reply
* Re: Are binary xdeltas only used if you use git-gc?
From: Thanassis Tsiodras @ 2008-10-31 11:16 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <20081031110245.GA22633@artemis.corp>
Actually, after using git-gc, git-repack isn't really needed...
git-gc identifies that the two files are very similar and re-deltifies
(see the du -s -k outputs in the original mail, after git-gc we have
in fact lower usage than the first commit).
My question is basically...
(a) why doesn't git detect this during commit and needs a git-gc
(b) whether after git-gc I would have seen the massive difference
during a subsequent git-push or not
Thanassis.
> Have you tried to git repack with aggressive options, like:
>
> git repack --window=500 --depth=500 \
> --window-memory=<fair amount of your physical RAM>
--
What I gave, I have; what I spent, I had; what I kept, I lost. -Old Epitaph
^ permalink raw reply
* Re: Are binary xdeltas only used if you use git-gc?
From: Jakub Narebski @ 2008-10-31 11:15 UTC (permalink / raw)
To: Thanassis Tsiodras; +Cc: git
In-Reply-To: <f1d2d9ca0810310243r669840bbj2c5ee7183e0caaed@mail.gmail.com>
"Thanassis Tsiodras" <ttsiodras@gmail.com> writes:
> I've been usig Git for the last couple of months and am quite happy with it.
> In one of my Git repositories, I am storing uncompressed .tar files
> (since being uncompressed allows git to detect and store
> only their "real"differences).
I think you can use clean / smudge filter in gitattributes for that.
[...]
> Then again, I must confess I only did the git-gc after I pushed.
> Does the git-push actually take advantage of the similarities only if
> I do a git-gc first?
Git does deltification _only_ in packfiles. But when you push via SSH
git would generate a pack file with commits the other side doesn't
have, and those packs are thin packs, so they also have deltas... but
the remote side then adds bases to those thin packs making them
standalone: you would have to git-gc on remote.
HTH
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH] Avoid using non-portable `echo-n` in tests.
From: Steve Folly @ 2008-10-31 9:36 UTC (permalink / raw)
To: git
In-Reply-To: <7vtzat89br.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster <at> pobox.com> writes:
>
> Brian Gernhardt <benji <at> silverinsanity.com> writes:
>
> > Not all /bin/sh have a builtin echo that recognizes -n. Using printf
> > is far more portable.
> >
> > Discovered on OS X 10.5.5 in t4030-diff-textconv.sh and changed in all
> > the test scripts.
>
> I had an impression that OS X was BSDish. Wasn't "echo -n" a BSDism?
>
FYI, "man echo" on OS X 10.5.5 has this to say...
The following option is available:
-n Do not print the trailing newline character. This may also be
achieved by appending `\c' to the end of the string, as is done by
iBCS2 compatible systems. Note that this option as well as the
effect of `\c' are implementation-defined in IEEE Std 1003.1-2001
(``POSIX.1'') as amended by Cor. 1-2002. Applications aiming for
maximum portability are strongly encouraged to use printf(1) to
suppress the newline character.
Some shells may provide a builtin echo command which is similar or iden-
tical to this utility. Most notably, the builtin echo in sh(1) does not
accept the -n option. Consult the builtin(1) manual page.
Regards,
Steve.
^ permalink raw reply
* Re: Are binary xdeltas only used if you use git-gc?
From: Pierre Habouzit @ 2008-10-31 11:02 UTC (permalink / raw)
To: Thanassis Tsiodras; +Cc: git
In-Reply-To: <f1d2d9ca0810310243r669840bbj2c5ee7183e0caaed@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 631 bytes --]
On Fri, Oct 31, 2008 at 09:43:43AM +0000, Thanassis Tsiodras wrote:
> So even though the xdelta is just 8KB, and git-gc actually finds out
> that indeed
> the new file is very similar to the old one, the initial commit of the
> new version
> in the repos is not taking advantage.
Have you tried to git repack with aggressive options, like:
git repack --window=500 --depth=500 \
--window-memory=<fair amount of your physical RAM>
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* git send-email improvements
From: Pierre Habouzit @ 2008-10-31 10:57 UTC (permalink / raw)
To: git
The teaser
==========
This series has been sent using:
git send-email --to git@vger.kernel.org --compose --annotate HEAD~3..
The series
==========
Here is a patch series to improve git send-email following our
discussions at GitTogether'08, despite my hate for perl.
The first patch is a minor nitpick, because leaking fd's sucks.
The second patch allow git-send-email to receive revision lists as
arguments. This doesn't allow complex arguments combinations as it
proces the revision lists one by one (IOW ^$sha1 $sha2 won't work as
expected _at all_) but this shouldn't be a problem since this command is
primarily used for interactive users. People wanting to use
git-send-email with complex revision lists through scripts MUST
git-format-patch first into a safe temporary directory and use
git-send-email on this afterwards.
The last patch adds the possibility to review patches into an editor
before sending them, which allow you (thanks to patch 2) to serialize,
review, annotate, and send patches in one command.
Further discussion
==================
I think one could make git send-email better doing this:
(1) make --compose and --annotate default, do not asking for a Subject
if it's missing, neither should we ask for the in-reply-to if it's
missing.
Then spawn the editor with a first empty file that contains rougly a
template looking like this:
----8<----
GIT: Purge this buffer from any content if you don't want a series summary
GIT:
GIT: Lines beginning in "GIT: " will be removed.
GIT: Consider including an overall diffstat or table of contents
GIT: for the patch you are writing.
GIT:
GIT: Please fill a Subject if missing
GIT: Leave the In-Reply-To field empty if not applicable.
Subject:
In-Reply-To:
--> <we may want to add some more headers here: To/Cc/Bcc/...>
GIT: put the content of the mail below this line
GIT: [PATCH 1/10] .... \
GIT: [PATCH 2/10] .... | this would contain all the Subject's
[...] | from the commits that are beeing sent
GIT: [PATCH 8/10] .... | as a conveniency for people not
GIT: [PATCH 9/10] .... | having to cut&paste them
GIT: [PATCH 10/10] .... /
---->8----
I suggest we don't enable --compose when the series is reduced to
one patch, as the usual way is to comment inline. This is probably
arguable.
(2) Introduce a --batch option that basically:
* turns --compose and --annotate off
* turns any interactive feature off
* complain (and fail) if it misses any information that is usually
asked interactively
What do you think ?
^ permalink raw reply
* [PATCH 3/3] git send-email: add --annotate option
From: Pierre Habouzit @ 2008-10-31 10:57 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <1225450632-7230-3-git-send-email-madcoder@debian.org>
This allows to review every patch (and fix various aspects of them, or
comment them) in an editor just before being sent. Combined to the fact
that git send-email can now process revision lists, this makes git
send-email and efficient way to review and send patches interactively.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
Documentation/git-send-email.txt | 11 +++++++++++
git-send-email.perl | 26 ++++++++++++++++++++++++--
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index cafff1a..9ee81d5 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -37,6 +37,11 @@ The --bcc option must be repeated for each user you want on the bcc list.
+
The --cc option must be repeated for each user you want on the cc list.
+--annotate::
+ Review each patch you're about to send in an editor. The setting
+ 'sendemail.multiedit' defines if this will spawn one editor per patch
+ or one for all of them at once.
+
--compose::
Use $GIT_EDITOR, core.editor, $VISUAL, or $EDITOR to edit an
introductory message for the patch series.
@@ -204,6 +209,12 @@ sendemail.aliasfiletype::
Format of the file(s) specified in sendemail.aliasesfile. Must be
one of 'mutt', 'mailrc', 'pine', or 'gnus'.
+sendemail.multiedit::
+ If true (default), a single editor instance will be spawned to edit
+ files you have to edit (patches when '--annotate' is used, and the
+ summary when '--compose' is used). If false, files will be edited one
+ after the other, spawning a new editor each time.
+
Author
------
diff --git a/git-send-email.perl b/git-send-email.perl
index 0d50ee2..65c254d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -48,6 +48,7 @@ git send-email [options] <file | directory | rev-list >
--bcc <str> * Email Bcc:
--subject <str> * Email "Subject:"
--in-reply-to <str> * Email "In-Reply-To:"
+ --annotate * Review each patch that will be sent in an editor.
--compose * Open an editor for introduction.
Sending:
@@ -130,7 +131,8 @@ my $compose_filename = ".msg.$$";
# Variables we fill in automatically, or via prompting:
my (@to,@cc,@initial_cc,@bcclist,@xh,
- $initial_reply_to,$initial_subject,@files,$author,$sender,$smtp_authpass,$compose,$time);
+ $initial_reply_to,$initial_subject,@files,
+ $author,$sender,$smtp_authpass,$annotate,$compose,$time);
my $envelope_sender;
@@ -151,6 +153,17 @@ if ($@) {
# Behavior modification variables
my ($quiet, $dry_run) = (0, 0);
+# Handle interactive edition of files.
+my $multiedit;
+my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
+sub do_edit {
+ if (defined($multiedit) && !$multiedit) {
+ map { system('sh', '-c', $editor.' "$@"', $editor, $_); } @_;
+ } else {
+ system('sh', '-c', $editor.' "$@"', $editor, @_);
+ }
+}
+
# Variables with corresponding config settings
my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);
my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);
@@ -180,6 +193,7 @@ my %config_settings = (
"aliasesfile" => \@alias_files,
"suppresscc" => \@suppress_cc,
"envelopesender" => \$envelope_sender,
+ "multiedit" => \$multiedit,
);
# Handle Uncouth Termination
@@ -222,6 +236,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
"smtp-ssl" => sub { $smtp_encryption = 'ssl' },
"smtp-encryption=s" => \$smtp_encryption,
"identity=s" => \$identity,
+ "annotate" => \$annotate,
"compose" => \$compose,
"quiet" => \$quiet,
"cc-cmd=s" => \$cc_cmd,
@@ -499,7 +514,12 @@ EOT
close(C);
my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
- system('sh', '-c', $editor.' "$@"', $editor, $compose_filename);
+
+ if ($annotate) {
+ do_edit($compose_filename, @files);
+ } else {
+ do_edit($compose_filename);
+ }
open(C2,">",$compose_filename . ".final")
or die "Failed to open $compose_filename.final : " . $!;
@@ -548,6 +568,8 @@ EOT
}
@files = ($compose_filename . ".final", @files);
+} elsif ($annotate) {
+ do_edit(@files);
}
# Variables we set as part of the loop over files
--
1.6.0.3.759.g40a2.dirty
^ permalink raw reply related
* [PATCH 2/3] git send-email: interpret unknown files as revision lists
From: Pierre Habouzit @ 2008-10-31 10:57 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <1225450632-7230-2-git-send-email-madcoder@debian.org>
Instead of skipping unkown files on the command line, pass them through
git format-patch into a safe temporary directory. This allow no
complicated rev-list option lists combining "--all" "--not" and so on, but
allow to use ranges which are quite enough for most of the use cases.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
Documentation/git-send-email.txt | 2 +-
git-send-email.perl | 6 ++++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 82f5056..cafff1a 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -8,7 +8,7 @@ git-send-email - Send a collection of patches as emails
SYNOPSIS
--------
-'git send-email' [options] <file|directory> [... file|directory]
+'git send-email' [options] <file|directory|rev-list>...
DESCRIPTION
diff --git a/git-send-email.perl b/git-send-email.perl
index 94ca5c8..0d50ee2 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -22,6 +22,7 @@ use Term::ReadLine;
use Getopt::Long;
use Data::Dumper;
use Term::ANSIColor;
+use File::Temp qw/ tempdir /;
use Git;
package FakeTerm;
@@ -38,7 +39,7 @@ package main;
sub usage {
print <<EOT;
-git send-email [options] <file | directory>...
+git send-email [options] <file | directory | rev-list >
Composing:
--from <str> * Email From:
@@ -378,7 +379,8 @@ for my $f (@ARGV) {
} elsif (-f $f or -p $f) {
push @files, $f;
} else {
- print STDERR "Skipping $f - not found.\n";
+ my $tempdir = tempdir(CLEANUP => 1);
+ push @files, $repo->command('format-patch', '-o', $tempdir, $f);
}
}
--
1.6.0.3.759.g40a2.dirty
^ permalink raw reply related
* [PATCH 1/3] git send-email: avoid leaking directory file descriptors.
From: Pierre Habouzit @ 2008-10-31 10:57 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <1225450632-7230-1-git-send-email-madcoder@debian.org>
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
git-send-email.perl | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index bdbfac6..94ca5c8 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -374,10 +374,9 @@ for my $f (@ARGV) {
push @files, grep { -f $_ } map { +$f . "/" . $_ }
sort readdir(DH);
-
+ closedir(DH);
} elsif (-f $f or -p $f) {
push @files, $f;
-
} else {
print STDERR "Skipping $f - not found.\n";
}
--
1.6.0.3.759.g40a2.dirty
^ 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