* Re: how to hide some branches
From: Alex Riesen @ 2008-11-30 15:16 UTC (permalink / raw)
To: pascal; +Cc: git list
In-Reply-To: <493261C9.4040608@obry.net>
2008/11/30 Pascal Obry <pascal@obry.net>:
> I create a new branch for every new feature/fix I work on. After some
> time I have many (too much) branch listed when doing:
>
> $ git branch
>
> I'd like to hide some (not removing them).
>
> Is there a solution for this?
>
> How you people handle this?
Rename them into something _not_ under refs/heads:
git update-ref refs/hidden/branch branch
git branch -D branch
?
^ permalink raw reply
* Managing websites with git
From: Felix Andersen @ 2008-11-30 16:30 UTC (permalink / raw)
To: git
Hi!
Is it a bad idea to manage websites (php/xhtml/css) by having a origin
non-bare repo in the hosted dir with the hook mentioned here:
http://git.or.cz/gitwiki/GitFaq#head-b96f48bc9c925074be9f95c0fce69bcece5f6e73.
I was thinking about any security issues with the .git dir being
hosted. Or is that even the right way to do it?
Thank you
Felix Andersen
^ permalink raw reply
* Re: Managing websites with git
From: David Bryson @ 2008-11-30 17:07 UTC (permalink / raw)
To: Felix Andersen; +Cc: git
In-Reply-To: <fe5a74300811300830x850d81csc5cf1f9b367bac11@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1112 bytes --]
Hello,
On Sun, Nov 30, 2008 at 05:30:30PM +0100 or thereabouts, Felix Andersen wrote:
> Hi!
>
> Is it a bad idea to manage websites (php/xhtml/css) by having a origin
> non-bare repo in the hosted dir with the hook mentioned here:
> http://git.or.cz/gitwiki/GitFaq#head-b96f48bc9c925074be9f95c0fce69bcece5f6e73.
> I was thinking about any security issues with the .git dir being
> hosted. Or is that even the right way to do it?
>
One really should not push to a non-bare repo. IIRC there was a patch
recently to disallow it, but I do not remember if it was merged into
HEAD.
Since I knew the patch was coming I rewrote my scripts to use a bare
repo in /var/git, and push the changes to /var/www whenever I push to
the remote repo.
I wrote my post-update to be something like the following.
#!/bin/bash
LIVE="/var/www/statichacks/blosxom"
ref=$1
cd $GIT_DIR
echo "Pushing updates to $LIVE..."
git archive --format=tar $ref | tar -C $LIVE --atime-preserve -xpf -
There may be an easier way to do it, but that script took me about 5
minutes to write and test.
Dave
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: Managing websites with git
From: Jeff King @ 2008-11-30 17:27 UTC (permalink / raw)
To: David Bryson; +Cc: Felix Andersen, git
In-Reply-To: <20081130170722.GJ6572@eratosthenes.sbcglobal.net>
On Sun, Nov 30, 2008 at 09:07:22AM -0800, David Bryson wrote:
> One really should not push to a non-bare repo. IIRC there was a patch
> recently to disallow it, but I do not remember if it was merged into
> HEAD.
It's in master and should be in 1.6.1, but it is a config option that
defaults to "warn" for now, so as not to break existing setups. It may
switch to "refuse" after a deprecation period, but I don't think the
length of that period has been set.
> Since I knew the patch was coming I rewrote my scripts to use a bare
> repo in /var/git, and push the changes to /var/www whenever I push to
> the remote repo.
Personally, I think that is a sane way to go; but note that you still
use a non-bare repo with a checkout hook by explicitly setting
receive.denyCurrentBranch to false.
> #!/bin/bash
> LIVE="/var/www/statichacks/blosxom"
>
> ref=$1
>
> cd $GIT_DIR
> echo "Pushing updates to $LIVE..."
> git archive --format=tar $ref | tar -C $LIVE --atime-preserve -xpf -
>
> There may be an easier way to do it, but that script took me about 5
> minutes to write and test.
One disadvantage of this method is that it doesn't remove files from
$LIVE that were deleted in the repository.
-Peff
^ permalink raw reply
* Re: [PATCH 4/5] upload-pack: implement protocol extension "symbolic-ref"
From: Jeff King @ 2008-11-30 18:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1228039053-31099-2-git-send-email-gitster@pobox.com>
On Sun, Nov 30, 2008 at 01:57:29AM -0800, Junio C Hamano wrote:
> Although the new capability is advertised on the first available ref in
> the same way as the other extensions, the way to trigger this extension
> from the receiving end is not by adding it in the first "want" line as
> usual. Instead, the receiving end sends a "symbolic-ref" request packet
> before the usual sequence of "want" lines.
>
> This is unfortunate because it forces an extra round trip (receiving end
> sends a "please tell me symbolic-ref" packet, and then upload side sends
> "here are the requested information" packet), but it has to be implemented
> this way because (1) ls-remote may need to ask for this information, in
> which case there is no "want" to be sent; and (2) the transport API
> insists that transport_get_remote_refs() returns the final list, and does
> not allow augmenting what was initially obtained from the call to it by
> later calls to transport_fetch_refs() easily.
Hrm. For (1), could we allow either interaction method? IOW, allow
requesting a symref on the first want line, _or_ by separate "symbolic
ref" packet? That would allow clients who are using "want" to
piggy-back the symref request as an optimization, but not restrict those
that just want to ask for it?
Not being too familiar with the transport code, I can't speak to (2).
But it would be sad to see an internal API shortcoming that we have
_now_ stick us with a crappy protocol _forever_. We can fix the API, but
once the protocol is in the wild, it becomes much harder to change.
> It also is unfortunate that with this change on the server side, older
> clients running "ls-remote" without actually downloading anything will
> trigger "The remote end hung up unexpectedly" error on the uploading side,
> which is annoying even though it is benign. You can observe it by applying
> only this patch but not the patch to the receiving end and running t5601
> under "sh -x".
And obviously this wouldn't go away with the proposal above, since we'd
still have to be looking for the "tell me this symbolic ref" packet. But
the solution you outlined in 0/5 sounded sane to me (and I think it
definitely needs to be addressed).
-Peff
^ permalink raw reply
* Re: [PATCH 3/5] clone: find the current branch more explicitly
From: Jeff King @ 2008-11-30 18:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1228039053-31099-3-git-send-email-gitster@pobox.com>
On Sun, Nov 30, 2008 at 01:57:30AM -0800, Junio C Hamano wrote:
> + sym = find_ref_by_name(refs, symref);
> + if (!sym) {
> + /*
> + * NEEDSWORK: perhaps create the symref ref
> + * that is still unborn and queue it?
> + */
> + continue;
> + }
I don't see any reason not to create the pointer to an unborn branch. I
think it would be nice eventually to allow cloning an empty repository
(i.e., to facilitate:
mkdir foo &&
(cd foo && git --bare init) &&
git clone foo bar &&
cd bar &&
echo content >file && git add file && git commit -m content &&
git push
) which people have asked for from time to time.
> + if (sym->symref)
> + die("symref line says %s points at %s "
> + "but earlier it said it points at %s",
> + symref, target, sym->symref);
I wonder if there would ever be a use for sending multiple symref
packets for the same ref. IOW, should we "be liberal in what we accept"
here and just choose some sane behavior (like picking the first or last
to be sent), and allow room for expansion there.
I can't actually think of a possible use for that, but nor can I think
of any particular reason to die here (save for detecting bugs in
upload-pack :) ). But just consider that we will be stuck with clients
with the "die" behavior forever.
-Peff
^ permalink raw reply
* Re: [PATCH 3/3] git add --intent-to-add: do not let an empty blob committed by accident
From: Jeff King @ 2008-11-30 19:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, Johannes Schindelin, git
In-Reply-To: <7vk5an2nil.fsf_-_@gitster.siamese.dyndns.org>
On Fri, Nov 28, 2008 at 07:56:34PM -0800, Junio C Hamano wrote:
> Subject: Re: [PATCH 3/3] git add --intent-to-add: do not let an empty blob
> committed by accident
Minor nit: grammatical error in the subject.
> This implies that you cannot "git commit" from such a state; however "git
> commit -a" still works.
I was going to provide a test for "git commit -a" to squash in, but it
looks like the version in 'pu' already has one.
> case WRITE_TREE_UNMERGED_INDEX:
> - die("%s: error building trees; the index is unmerged?", me);
> + die("%s: error building trees", me);
This caught me by surprise while reading, but I assume the rationale is
"now there is a new, different reason not to be able to build the trees,
so our guess is less likely to be correct". I wonder if we can do better
by actually passing out a more exact error value (though it looks like
we will already have said "foo: not added yet" by that point anyway, so
maybe it is just pointless to say more).
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
Why in t3701? These don't have anything to do with interactive add, and
there is a already a t2203-add-intent.
-Peff
^ permalink raw reply
* Re: [PATCH 2/3] git add --intent-to-add: fix removal of cached emptiness
From: Jeff King @ 2008-11-30 19:21 UTC (permalink / raw)
To: sverre; +Cc: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, git
In-Reply-To: <bd6139dc0811290738qbd93ff6oa7aa854708009075@mail.gmail.com>
On Sat, Nov 29, 2008 at 04:38:12PM +0100, Sverre Rabbelier wrote:
> On Sat, Nov 29, 2008 at 04:55, Junio C Hamano <gitster@pobox.com> wrote:
> > This uses the extended index flag mechanism introduced earlier to mark
> > the entries added to the index via "git add -N" with CE_INTENT_TO_ADD.
>
> Is 'intent' [0] used properly here? Should it not be 'intend' [1]?
>
> [0] http://en.wiktionary.org/wiki/intent
> [1] http://en.wiktionary.org/wiki/intend
I think it's fine. The flags describe the entry, not the user (e.g.,
CE_VALID). So the entry does not _intend_ to add anything, but rather
there exists _intent_ to add this entry (you might also say the entry is
"_intended_ to be added", but that is getting a bit clunky).
-Peff
^ permalink raw reply
* Re: how to hide some branches
From: Pete Harlan @ 2008-11-30 20:09 UTC (permalink / raw)
To: pascal; +Cc: git list
In-Reply-To: <493261C9.4040608@obry.net>
Pascal Obry wrote:
> Hello everyone,
>
> I create a new branch for every new feature/fix I work on. After some
> time I have many (too much) branch listed when doing:
>
> $ git branch
>
> I'd like to hide some (not removing them).
>
> Is there a solution for this?
>From a suggestion by Jakub Narebski, I use an alias "lb" that shows
the most recently-active 8 branches:
for-each-ref --format='%(refname:short)' \
--sort=-authordate --count=8 refs/heads/
--Pete
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)
From: Daniel Barkalow @ 2008-11-30 21:26 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, git
In-Reply-To: <fcaeb9bf0811300229v4e7bfbb7g9a0ac72dcddb4326@mail.gmail.com>
On Sun, 30 Nov 2008, Nguyen Thai Ngoc Duy wrote:
> On 11/29/08, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> > On 11/29/08, Daniel Barkalow <barkalow@iabervon.org> wrote:
> > > If there's any need for this to be distinguished from "assume unchanged",
> > > I think it should be used with, not instead of, the CE_VALID bit; and it
> > > could probably use some bit in the stat info section, since we don't need
> > > stat info if we know by assumption that the entry is valid.
> >
> >
> > Interesting. I'll think more about this.
> >
>
> As I said, CE_VALID implies all files are present.
My first question is whether this actually should be true. Going back to
the message for 5f73076c1a9b4b8dc94f77eac98eb558d25e33c0, it sounds like
the CE_VALID code is designed to be safe and sort of correct even if the
files are not actually unchanged; I don't think it would be out-of-spec
for CE_VALID to (1) always produce output as if the working tree contained
what the index contains, while (2) refusing to make any changes to working
tree files that do not actually match the index. As it is now, (2) is
explicitly true, but (1) is left vague-- commands may fail entirely or
produce different output if CE_VALID is set in the index for a file that
has changes in the working tree, but not in any particular way.
Now, it might be necessary for CE_NO_CHECKOUT to differ from CE_VALID in
some ways in (2): if a file is CE_NO_CHECKOUT and absent, code which would
modify it could probably just report sucess, while CE_VALID on a file
with changes should probably report failure. On the other hand, that could
just as easily be at the porcelain layer, with the porcelain instructing
the plumbing to change the index without changing the working tree for
those files outside the sparse checkout, and the plumbing would report
errors if the porcelain did not do this.
> I could make CE_NO_CHECKOUT to be used with CE_VALID, but I would need
> to check all CE_VALID code path to make sure the behaviour remains if
> CE_NO_CHECKOUT is absent. It's just more intrusive.
I would expect all code that has a CE_VALID path to do something actually
wrong if it took the non-CE_VALID code path on CE_NO_CHECKOUT and there
was no CE_NO_CHECKOUT code path. So I'd expect that your patch is
insufficient to the extent that CE_NO_CHECKOUT doesn't imply CE_VALID
(since there is very little in the way of CE_NO_CHECKOUT-specific
handling in your patch).
The only case I can think of where NO_CHECKOUT is more like !VALID than
VALID is with respect to whether we can report the content in the index by
looking in the filesystem instead of in the database; I don't think this
is an intentional optimization anywhere, and I think it would be a likely
source of bugs if it were (e.g., it would have to know about files which
are up-to-date with respect to stat info, but which have been "smudged" on
disk and therefore don't match byte-for-byte with the database). Actually,
it might be most accurate to treat --no-checkout as being CE_VALID with a
smudge filter of "rm". If the combination of CE_VALID and on-disk
conversion works (which is likely to be the common pattern for Windows
users, who need autocrlf and have a slow lstat(), and is therefore
maintained), surely this combination would work for CE_NO_CHECKOUT.
> I have nothing against storing CE_NO_CHECKOUT in stat info except that
> it seems inappropriate/hidden place to do. ce_flags is more obvious
> choice. I haven't looked closely to stat info code in read-cache.c
> though.
It should be pretty clean to check CE_VALID when reading an entry from
disk and remap bits from it to additional flags in memory. I wouldn't
suggest overlaying them in memory, but there's also no shortage of space
for flags in memory.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* [JGIT PATCH v2 0/8] Unit test cleanups
From: Robin Rosenberg @ 2008-11-30 23:40 UTC (permalink / raw)
To: spearce; +Cc: git, fonseca, Robin Rosenberg
In-Reply-To: <20081127214916.GD23984@spearce.org>
A completele reworked set of patches, including fixing a couple
more forgot-to-close bugs and Shawns suggestion that we disable
memory mapping in junit tests by default.
-- robin
Robin Rosenberg (8):
Drop unneeded code in unit tests
Cleanup malformed test cases
Turn off memory mapping in JGit unit tests by default
Add a counter to make sure the test repo name is unique
Make the cleanup less verbose when it fails to delete temporary
stuff.
Cleanup after each test.
Close files opened by unit testing framework
Hard failure on unit test cleanups if they fail.
.../tst/org/spearce/jgit/lib/PackWriterTest.java | 3 +
.../org/spearce/jgit/lib/RepositoryTestCase.java | 152 +++++++++++++++++---
.../tst/org/spearce/jgit/lib/T0007_Index.java | 10 +-
3 files changed, 139 insertions(+), 26 deletions(-)
^ permalink raw reply
* [JGIT PATCH 1/8] Drop unneeded code in unit tests
From: Robin Rosenberg @ 2008-11-30 23:40 UTC (permalink / raw)
To: spearce; +Cc: git, fonseca, Robin Rosenberg
In-Reply-To: <1228088435-23722-1-git-send-email-robin.rosenberg@dewire.com>
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../org/spearce/jgit/lib/RepositoryTestCase.java | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
index 9d7d133..e164faf 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
@@ -172,8 +172,6 @@ protected void tearDown() throws Exception {
protected Repository createNewEmptyRepo() throws IOException {
File newTestRepo = new File(trashParent, "new"+System.currentTimeMillis()+"/.git");
assertFalse(newTestRepo.exists());
- File unusedDir = new File(trashParent, "tmp"+System.currentTimeMillis());
- assertTrue(unusedDir.mkdirs());
final Repository newRepo = new Repository(newTestRepo);
newRepo.create();
return newRepo;
--
1.6.0.3.640.g6331a
^ permalink raw reply related
* [JGIT PATCH 3/8] Turn off memory mapping in JGit unit tests by default
From: Robin Rosenberg @ 2008-11-30 23:40 UTC (permalink / raw)
To: spearce; +Cc: git, fonseca, Robin Rosenberg
In-Reply-To: <1228088435-23722-3-git-send-email-robin.rosenberg@dewire.com>
A system property named jgit.junit.usemmmap can be set to true to enable
memory mapping during unit testing.
The protected method configure can be overridden to do things
like configuring the JGit engine.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../org/spearce/jgit/lib/RepositoryTestCase.java | 30 ++++++++++++++++++++
1 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
index e164faf..3b08fa5 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
@@ -49,6 +49,19 @@
import junit.framework.TestCase;
import org.spearce.jgit.util.JGitTestUtil;
+/**
+ * Base class for most JGit unit tests.
+ *
+ * Sets up a predefined test repository and has support for creating additional
+ * repositories and destroying them when the tests are finished.
+ *
+ * A system property <em>jgit.junit.usemmmap</em> defines whether memory mapping
+ * is used. Memory mapping has an effect on the file system, in that memory
+ * mapped files in java cannot be deleted as long as they mapped arrays have not
+ * been reclaimed by the garbage collector. The programmer cannot control this
+ * with precision, though hinting using <em>{@link java.lang.System#gc}</em>
+ * often helps.
+ */
public abstract class RepositoryTestCase extends TestCase {
protected final File trashParent = new File("trash");
@@ -66,6 +79,22 @@
jcommitter = new PersonIdent("J. Committer", "jcommitter@example.com");
}
+ protected boolean packedGitMMAP;
+
+ /**
+ * Configure JGit before setting up test repositories.
+ */
+ protected void configure() {
+ packedGitMMAP = "true".equals(System.getProperty("jgit.junit.usemmmap"));
+ WindowCache.reconfigure(128*1024, 8192, packedGitMMAP, 8192);
+ }
+
+ /**
+ * Utility method to delete a directory recursively. It is
+ * also used internally.
+ *
+ * @param dir
+ */
protected static void recursiveDelete(final File dir) {
final File[] ls = dir.listFiles();
if (ls != null) {
@@ -123,6 +152,7 @@ protected static void checkFile(File f, final String checkData)
public void setUp() throws Exception {
super.setUp();
+ configure();
recursiveDelete(trashParent);
trash = new File(trashParent,"trash"+System.currentTimeMillis());
trash_git = new File(trash, ".git");
--
1.6.0.3.640.g6331a
^ permalink raw reply related
* [JGIT PATCH 2/8] Cleanup malformed test cases
From: Robin Rosenberg @ 2008-11-30 23:40 UTC (permalink / raw)
To: spearce; +Cc: git, fonseca, Robin Rosenberg
In-Reply-To: <1228088435-23722-2-git-send-email-robin.rosenberg@dewire.com>
These were abusing setup, resulting in lost resources. To
fix this we can abuse tearDown too.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../tst/org/spearce/jgit/lib/PackWriterTest.java | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java
index e5bce4d..3c02955 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java
@@ -309,6 +309,7 @@ public void testWritePack4ThinPack() throws IOException {
public void testWritePack2SizeDeltasVsNoDeltas() throws Exception {
testWritePack2();
final int sizePack2NoDeltas = cos.getCount();
+ tearDown();
setUp();
testWritePack2DeltasReuseRefs();
final int sizePack2DeltasRefs = cos.getCount();
@@ -327,6 +328,7 @@ public void testWritePack2SizeDeltasVsNoDeltas() throws Exception {
public void testWritePack2SizeOffsetsVsRefs() throws Exception {
testWritePack2DeltasReuseRefs();
final int sizePack2DeltasRefs = cos.getCount();
+ tearDown();
setUp();
testWritePack2DeltasReuseOffsets();
final int sizePack2DeltasOffsets = cos.getCount();
@@ -344,6 +346,7 @@ public void testWritePack2SizeOffsetsVsRefs() throws Exception {
public void testWritePack4SizeThinVsNoThin() throws Exception {
testWritePack4();
final int sizePack4 = cos.getCount();
+ tearDown();
setUp();
testWritePack4ThinPack();
final int sizePack4Thin = cos.getCount();
--
1.6.0.3.640.g6331a
^ permalink raw reply related
* [JGIT PATCH 4/8] Add a counter to make sure the test repo name is unique
From: Robin Rosenberg @ 2008-11-30 23:40 UTC (permalink / raw)
To: spearce; +Cc: git, fonseca, Robin Rosenberg
In-Reply-To: <1228088435-23722-4-git-send-email-robin.rosenberg@dewire.com>
System.currentTimeMillis() does not have the granularity
necessary to guarantee uniqueness. We keep it to make sure we
have unique names between different runs, but add a counter to
make it unique within the execution of a test suite.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../org/spearce/jgit/lib/RepositoryTestCase.java | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
index 3b08fa5..6ea9b45 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
@@ -150,11 +150,13 @@ protected static void checkFile(File f, final String checkData)
protected Repository db;
+ private static int testcount;
+
public void setUp() throws Exception {
super.setUp();
configure();
recursiveDelete(trashParent);
- trash = new File(trashParent,"trash"+System.currentTimeMillis());
+ trash = new File(trashParent,"trash"+System.currentTimeMillis()+"."+(testcount++));
trash_git = new File(trash, ".git");
Runtime.getRuntime().addShutdownHook(new Thread() {
@@ -200,7 +202,7 @@ protected void tearDown() throws Exception {
* @throws IOException
*/
protected Repository createNewEmptyRepo() throws IOException {
- File newTestRepo = new File(trashParent, "new"+System.currentTimeMillis()+"/.git");
+ File newTestRepo = new File(trashParent, "new"+System.currentTimeMillis()+"."+(testcount++)+"/.git");
assertFalse(newTestRepo.exists());
final Repository newRepo = new Repository(newTestRepo);
newRepo.create();
--
1.6.0.3.640.g6331a
^ permalink raw reply related
* [JGIT PATCH 7/8] Close files opened by unit testing framework
From: Robin Rosenberg @ 2008-11-30 23:40 UTC (permalink / raw)
To: spearce; +Cc: git, fonseca, Robin Rosenberg
In-Reply-To: <1228088435-23722-7-git-send-email-robin.rosenberg@dewire.com>
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../org/spearce/jgit/lib/RepositoryTestCase.java | 12 ++++++++----
.../tst/org/spearce/jgit/lib/T0007_Index.java | 10 +++++++---
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
index aaa3592..376a76e 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
@@ -165,10 +165,14 @@ protected File writeTrashFile(final String name, final String data)
protected static void checkFile(File f, final String checkData)
throws IOException {
Reader r = new InputStreamReader(new FileInputStream(f), "ISO-8859-1");
- char[] data = new char[(int) f.length()];
- if (f.length() != r.read(data))
- throw new IOException("Internal error reading file data from "+f);
- assertEquals(checkData, new String(data));
+ try {
+ char[] data = new char[(int) f.length()];
+ if (f.length() != r.read(data))
+ throw new IOException("Internal error reading file data from "+f);
+ assertEquals(checkData, new String(data));
+ } finally {
+ r.close();
+ }
}
protected Repository db;
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0007_Index.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0007_Index.java
index 69f3a48..499812e 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0007_Index.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0007_Index.java
@@ -424,9 +424,13 @@ public void test031_executeBit_coreModeFalse() throws IllegalArgumentException,
private String content(File f) throws IOException {
byte[] buf = new byte[(int) f.length()];
FileInputStream is = new FileInputStream(f);
- int read = is.read(buf);
- assertEquals(f.length(), read);
- return new String(buf, 0);
+ try {
+ int read = is.read(buf);
+ assertEquals(f.length(), read);
+ return new String(buf, 0);
+ } finally {
+ is.close();
+ }
}
private void delete(File f) throws IOException {
--
1.6.0.3.640.g6331a
^ permalink raw reply related
* [JGIT PATCH 5/8] Make the cleanup less verbose when it fails to delete temporary stuff.
From: Robin Rosenberg @ 2008-11-30 23:40 UTC (permalink / raw)
To: spearce; +Cc: git, fonseca, Robin Rosenberg
In-Reply-To: <1228088435-23722-5-git-send-email-robin.rosenberg@dewire.com>
Pass the test case name for easier tracking of the test case that
causes problems.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../org/spearce/jgit/lib/RepositoryTestCase.java | 36 ++++++++++++++++----
1 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
index 6ea9b45..8e23bc1 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
@@ -96,21 +96,42 @@ protected void configure() {
* @param dir
*/
protected static void recursiveDelete(final File dir) {
+ recursiveDelete(dir, false, null);
+ }
+
+ protected static boolean recursiveDelete(final File dir, boolean silent,
+ final String name) {
+ if (!dir.exists())
+ return silent;
final File[] ls = dir.listFiles();
if (ls != null) {
for (int k = 0; k < ls.length; k++) {
final File e = ls[k];
if (e.isDirectory()) {
- recursiveDelete(e);
+ silent = recursiveDelete(e, silent, name);
} else {
- e.delete();
+ if (!e.delete()) {
+ if (!silent) {
+ String msg = "Warning: Failed to delete " + e;
+ if (name != null)
+ msg += " in " + name;
+ System.out.println(msg);
+ }
+ silent = true;
+ }
}
}
}
- dir.delete();
- if (dir.exists()) {
- System.out.println("Warning: Failed to delete " + dir);
+ if (!dir.delete()) {
+ if (!silent) {
+ String msg = "Warning: Failed to delete " + dir;
+ if (name != null)
+ msg += " in " + name;
+ System.out.println(msg);
+ }
+ silent = true;
}
+ return silent;
}
protected static void copyFile(final File src, final File dst)
@@ -155,14 +176,15 @@ protected static void checkFile(File f, final String checkData)
public void setUp() throws Exception {
super.setUp();
configure();
- recursiveDelete(trashParent);
+ final String name = getClass().getName() + "." + getName();
+ recursiveDelete(trashParent, true, name);
trash = new File(trashParent,"trash"+System.currentTimeMillis()+"."+(testcount++));
trash_git = new File(trash, ".git");
Runtime.getRuntime().addShutdownHook(new Thread() {
@Override
public void run() {
- recursiveDelete(trashParent);
+ recursiveDelete(trashParent, false, name);
}
});
--
1.6.0.3.640.g6331a
^ permalink raw reply related
* [JGIT PATCH 6/8] Cleanup after each test.
From: Robin Rosenberg @ 2008-11-30 23:40 UTC (permalink / raw)
To: spearce; +Cc: git, fonseca, Robin Rosenberg
In-Reply-To: <1228088435-23722-6-git-send-email-robin.rosenberg@dewire.com>
Automatically clean up any repositories created by the test cases.
Cleanup is attempted at the end of each test, but if that fails
Shutdown hooks attempt to clean up when the JVM exits. If memmory
mapping is enabled (disabled by default in unit tests), gc is
invoked to make it more likely that cleanup will occur successfully.
The drawback is that this is much slower, which is the reason we
disble memory mapping by default in unit tests.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../org/spearce/jgit/lib/RepositoryTestCase.java | 62 ++++++++++++++++----
1 files changed, 50 insertions(+), 12 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
index 8e23bc1..aaa3592 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
@@ -45,6 +45,8 @@
import java.io.InputStreamReader;
import java.io.OutputStreamWriter;
import java.io.Reader;
+import java.util.ArrayList;
+import java.util.List;
import junit.framework.TestCase;
import org.spearce.jgit.util.JGitTestUtil;
@@ -95,8 +97,8 @@ protected void configure() {
*
* @param dir
*/
- protected static void recursiveDelete(final File dir) {
- recursiveDelete(dir, false, null);
+ protected void recursiveDelete(final File dir) {
+ recursiveDelete(dir, false, getClass().getName() + "." + getName());
}
protected static boolean recursiveDelete(final File dir, boolean silent,
@@ -170,9 +172,12 @@ protected static void checkFile(File f, final String checkData)
}
protected Repository db;
-
+ private static Thread shutdownhook;
+ private static List<Runnable> shutDownCleanups = new ArrayList<Runnable>();
private static int testcount;
+ private ArrayList<Repository> repositoriesToClose = new ArrayList<Repository>();
+
public void setUp() throws Exception {
super.setUp();
configure();
@@ -180,14 +185,23 @@ public void setUp() throws Exception {
recursiveDelete(trashParent, true, name);
trash = new File(trashParent,"trash"+System.currentTimeMillis()+"."+(testcount++));
trash_git = new File(trash, ".git");
-
- Runtime.getRuntime().addShutdownHook(new Thread() {
- @Override
- public void run() {
- recursiveDelete(trashParent, false, name);
- }
- });
-
+ if (shutdownhook == null) {
+ shutdownhook = new Thread() {
+ @Override
+ public void run() {
+ // This may look superfluous, but is an extra attempt
+ // to clean up. First GC to release as many resources
+ // as possible and then try to clean up one test repo
+ // at a time (to record problems) and finally to drop
+ // the directory containing all test repositories.
+ System.gc();
+ for (Runnable r : shutDownCleanups)
+ r.run();
+ recursiveDelete(trashParent, false, null);
+ }
+ };
+ Runtime.getRuntime().addShutdownHook(shutdownhook);
+ }
db = new Repository(trash_git);
db.create();
@@ -213,6 +227,22 @@ copyFile(JGitTestUtil.getTestResourceFile(packs[k] + ".idx"), new File(packDir,
protected void tearDown() throws Exception {
db.close();
+ for (Repository r : repositoriesToClose)
+ r.close();
+
+ // Since memory mapping is controlled by the GC we need to
+ // tell it this is a good time to clean up and unlock
+ // memory mapped files.
+ if (packedGitMMAP)
+ System.gc();
+
+ final String name = getClass().getName() + "." + getName();
+ recursiveDelete(trash, false, name);
+ for (Repository r : repositoriesToClose)
+ recursiveDelete(r.getWorkDir(), false, name);
+
+ repositoriesToClose.clear();
+
super.tearDown();
}
@@ -224,10 +254,18 @@ protected void tearDown() throws Exception {
* @throws IOException
*/
protected Repository createNewEmptyRepo() throws IOException {
- File newTestRepo = new File(trashParent, "new"+System.currentTimeMillis()+"."+(testcount++)+"/.git");
+ final File newTestRepo = new File(trashParent, "new"
+ + System.currentTimeMillis() + "." + (testcount++) + "/.git");
assertFalse(newTestRepo.exists());
final Repository newRepo = new Repository(newTestRepo);
newRepo.create();
+ final String name = getClass().getName() + "." + getName();
+ shutDownCleanups.add(new Runnable() {
+ public void run() {
+ recursiveDelete(newTestRepo, false, name);
+ }
+ });
+ repositoriesToClose.add(newRepo);
return newRepo;
}
--
1.6.0.3.640.g6331a
^ permalink raw reply related
* [JGIT PATCH 8/8] Hard failure on unit test cleanups if they fail.
From: Robin Rosenberg @ 2008-11-30 23:40 UTC (permalink / raw)
To: spearce; +Cc: git, fonseca, Robin Rosenberg
In-Reply-To: <1228088435-23722-8-git-send-email-robin.rosenberg@dewire.com>
This only has an effect on Windows that locks open files, but
is a nice test that we actually clean up.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../org/spearce/jgit/lib/RepositoryTestCase.java | 52 ++++++++++++--------
1 files changed, 32 insertions(+), 20 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
index 376a76e..22bf395 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
@@ -49,6 +49,7 @@
import java.util.List;
import junit.framework.TestCase;
+
import org.spearce.jgit.util.JGitTestUtil;
/**
@@ -93,16 +94,18 @@ protected void configure() {
/**
* Utility method to delete a directory recursively. It is
- * also used internally.
+ * also used internally. If a file or directory cannote be removed
+ * it throws an AssertionFailure.
*
* @param dir
*/
protected void recursiveDelete(final File dir) {
- recursiveDelete(dir, false, getClass().getName() + "." + getName());
+ recursiveDelete(dir, false, getClass().getName() + "." + getName(), true);
}
protected static boolean recursiveDelete(final File dir, boolean silent,
- final String name) {
+ final String name, boolean failOnError) {
+ assert !(silent && failOnError);
if (!dir.exists())
return silent;
final File[] ls = dir.listFiles();
@@ -110,32 +113,42 @@ protected static boolean recursiveDelete(final File dir, boolean silent,
for (int k = 0; k < ls.length; k++) {
final File e = ls[k];
if (e.isDirectory()) {
- silent = recursiveDelete(e, silent, name);
+ silent = recursiveDelete(e, silent, name, failOnError);
} else {
if (!e.delete()) {
if (!silent) {
- String msg = "Warning: Failed to delete " + e;
- if (name != null)
- msg += " in " + name;
- System.out.println(msg);
+ reportDeleteFailure(name, failOnError, e);
}
- silent = true;
+ silent = !failOnError;
}
}
}
}
if (!dir.delete()) {
if (!silent) {
- String msg = "Warning: Failed to delete " + dir;
- if (name != null)
- msg += " in " + name;
- System.out.println(msg);
+ reportDeleteFailure(name, failOnError, dir);
}
- silent = true;
+ silent = !failOnError;
}
return silent;
}
+ private static void reportDeleteFailure(final String name,
+ boolean failOnError, final File e) {
+ String severity;
+ if (failOnError)
+ severity = "Error";
+ else
+ severity = "Warning";
+ String msg = severity + ": Failed to delete " + e;
+ if (name != null)
+ msg += " in " + name;
+ if (failOnError)
+ fail(msg);
+ else
+ System.out.println(msg);
+ }
+
protected static void copyFile(final File src, final File dst)
throws IOException {
final FileInputStream fis = new FileInputStream(src);
@@ -186,7 +199,7 @@ public void setUp() throws Exception {
super.setUp();
configure();
final String name = getClass().getName() + "." + getName();
- recursiveDelete(trashParent, true, name);
+ recursiveDelete(trashParent, true, name, false); // Cleanup old failed stuff
trash = new File(trashParent,"trash"+System.currentTimeMillis()+"."+(testcount++));
trash_git = new File(trash, ".git");
if (shutdownhook == null) {
@@ -201,7 +214,7 @@ public void run() {
System.gc();
for (Runnable r : shutDownCleanups)
r.run();
- recursiveDelete(trashParent, false, null);
+ recursiveDelete(trashParent, false, null, false);
}
};
Runtime.getRuntime().addShutdownHook(shutdownhook);
@@ -241,10 +254,9 @@ protected void tearDown() throws Exception {
System.gc();
final String name = getClass().getName() + "." + getName();
- recursiveDelete(trash, false, name);
+ recursiveDelete(trash, false, name, true);
for (Repository r : repositoriesToClose)
- recursiveDelete(r.getWorkDir(), false, name);
-
+ recursiveDelete(r.getWorkDir(), false, name, true);
repositoriesToClose.clear();
super.tearDown();
@@ -266,7 +278,7 @@ protected Repository createNewEmptyRepo() throws IOException {
final String name = getClass().getName() + "." + getName();
shutDownCleanups.add(new Runnable() {
public void run() {
- recursiveDelete(newTestRepo, false, name);
+ recursiveDelete(newTestRepo, false, name, false);
}
});
repositoriesToClose.add(newRepo);
--
1.6.0.3.640.g6331a
^ permalink raw reply related
* Re: [PATCH 0/2] gitweb: patch view
From: Jakub Narebski @ 2008-12-01 0:45 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <cb7bb73a0811291744t2bb9c8c1t1dac497705e2c3c2@mail.gmail.com>
On Sun, 30 Nov 2008, Giuseppe Bilotta wrote:
> On Sun, Nov 30, 2008 at 2:06 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Sat, 29 Nov 2008, Giuseppe Bilotta wrote:
>>
>>> I recently discovered that the commitdiff_plain view is not exactly
>>> something that can be used by git am directly (for example, the subject
>>> line gets duplicated in the commit message body after using git am).
>>
>> That's because gitweb generates email-like format "by hand", instead
>> of using '--format=email' or git-format-patch like in your series. On
>> the other hand that allows us to add extra headers, namely X-Git-Tag:
>> (which hasn't best implementation, anyway) and X-Git-Url: with URL
>> for given output.
>
>> By the way, we still might want to add somehow X-Git-Url and X-Git-Tag
>> headers later to 'patch' ('patchset') output format.
>
> Yeah, I've been thinking about it, but I couldn't find an easy and
> robust way to do it. Plus, should we add them for each patch, or just
> once for the whole patchset?
True, that is a complication. Perhaps they should be added only for
single patch?
>>> Since I'm not sure if it was the case to fix the plain view because I
>>> don't know what its intended usage was, I prepared a new view,
>>> uncreatively called 'patch', that exposes git format-patch output
>>> directly.
>>
>> Perhaps 'format_patch' would be better... hmmm... ?
>
> Considering I think commitdiff is ugly and long, you can guess my
> opinion on format_patch 8-P. 'patchset' might be a good candidate,
> considering it's what it does when both hash_parent and hash are
> given.
True, 'patchset' might be even better, especially that it hints
what it does for a range a..b (not diff of endpoints, but series
of patches).
>> Actually IMHO both 'commitdiff' and 'commitdiff_plain' try to do two
>> things at once. First to show diff _for_ a commit, i.e. equivalent of
>> "git show" or "git show --pretty=email", perhaps choosing one of
>> parents for a merge commit. Then showing commit message for $hash has
>> sense. The fact that 'commit' view doesn't show patchset, while
>> 'commitdiff' does might be result of historical situation.
>>
>> Second, to show diff _between_ commits, i.e. equivalent of
>> "git diff branch master". Then there doesn't make much sense to show
>> full commit message _only_ for one side of diff. IMHO that should be
>> main purpose of 'commitdiff' and 'commitdiff_plain' views, or simply
>> 'diff' / 'diff_plain' future views.
>
> We can probably consider deprecating commitdiff(_plain) and have the
> following three views:
>
> * commit(_plain): do what commitdiff(_plain) currently does for a single commit
Equivalent of "git show" (and not merely "git cat-file -t commit").
> * diff(_plain): do what commitdiff(_plain) currently does for
> parent..hash views, modulo something to be discussed for commit
> messages (a shortlog rather maybe?)
Equivalent of "git diff" (or "git diff-tree").
Diffstat, or dirstat might be a good idea. Shortlog... I am not sure.
Diff is about endpoints, and they can be in reverse, too.
There is a problem how to denote endpoints.
> * patch[set?][_plain?]: format-patch style output (maybe with option
> for HTML stuff too)
Equivalent of "git format-patch".
Actually the HTML format would be more like "git log -p", so perhaps
that could be handled simply as a version of 'log' view (perhaps via
@extra_options aka 'opt' parameter).
>> What 'patch' view does, what might be not obvious from this description
>> and from first patch in series, is to show diffs for _series_ of
>> commits. It means equivalent of "git log -p" or "git whatchanged".
>> It might make more sense to have plain git-format-patch output, but it
>> could be useful to have some kind of 'git log -p' HTML output.
>>
>> So even if 'commitdiff' / 'commitdiff_plain' is fixed, 'patch' whould
>> still have its place.
>
> Nice to know. Do consider the current version more of a
> proof-of-concept that some definitive code.
Ah. O.K. It would be nice if this patch was marked as RFC (well, lack
of signoff hints at this), or as WIP, or as PoC,...
>>> The second patch exposes it from commitdiff view (obviosly), but also
>>> from shortlog view, when less than 16 patches are begin shown.
>>
>> Why this nonconfigurable limit?
>
> Because the patch was actually a quick hack for the proof of concept
> 8-) I wasn't even sure the patch idea would have been worth it (as
> opposed to email-izing commitdiff_plain).
Ah.
Well, we might want to impose some limit to avoid generating and sending
patchset for a whole history. Perhaps to page size (100), or some similar
number?
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 0/2] gitweb: patch view
From: Giuseppe Bilotta @ 2008-12-01 1:10 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <200812010145.36612.jnareb@gmail.com>
On Mon, Dec 1, 2008 at 1:45 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Sun, 30 Nov 2008, Giuseppe Bilotta wrote:
>> On Sun, Nov 30, 2008 at 2:06 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>>> On Sat, 29 Nov 2008, Giuseppe Bilotta wrote:
>>
>>> By the way, we still might want to add somehow X-Git-Url and X-Git-Tag
>>> headers later to 'patch' ('patchset') output format.
>>
>> Yeah, I've been thinking about it, but I couldn't find an easy and
>> robust way to do it. Plus, should we add them for each patch, or just
>> once for the whole patchset?
>
> True, that is a complication. Perhaps they should be added only for
> single patch?
Although that's rather easy to implement technically, it also creates
some kind of inconsistency.
>> Considering I think commitdiff is ugly and long, you can guess my
>> opinion on format_patch 8-P. 'patchset' might be a good candidate,
>> considering it's what it does when both hash_parent and hash are
>> given.
>
> True, 'patchset' might be even better, especially that it hints
> what it does for a range a..b (not diff of endpoints, but series
> of patches).
Good, I'll rename it.
>> We can probably consider deprecating commitdiff(_plain) and have the
>> following three views:
>>
>> * commit(_plain): do what commitdiff(_plain) currently does for a single commit
>
> Equivalent of "git show" (and not merely "git cat-file -t commit").
>
>> * diff(_plain): do what commitdiff(_plain) currently does for
>> parent..hash views, modulo something to be discussed for commit
>> messages (a shortlog rather maybe?)
>
> Equivalent of "git diff" (or "git diff-tree").
>
> Diffstat, or dirstat might be a good idea. Shortlog... I am not sure.
> Diff is about endpoints, and they can be in reverse, too.
>
> There is a problem how to denote endpoints.
Hm? Doesn't parent..hash work? Or are you talking about something else?
>> * patch[set?][_plain?]: format-patch style output (maybe with option
>> for HTML stuff too)
>
> Equivalent of "git format-patch".
>
> Actually the HTML format would be more like "git log -p", so perhaps
> that could be handled simply as a version of 'log' view (perhaps via
> @extra_options aka 'opt' parameter).
This is starting to get complicated ... I'm not sure how far in this I
can go with this patchset, so for the time being I'll probably just
stick to refining the (plain) patchset feature.
>>> What 'patch' view does, what might be not obvious from this description
>>> and from first patch in series, is to show diffs for _series_ of
>>> commits. It means equivalent of "git log -p" or "git whatchanged".
>>> It might make more sense to have plain git-format-patch output, but it
>>> could be useful to have some kind of 'git log -p' HTML output.
>>>
>>> So even if 'commitdiff' / 'commitdiff_plain' is fixed, 'patch' whould
>>> still have its place.
>>
>> Nice to know. Do consider the current version more of a
>> proof-of-concept that some definitive code.
>
> Ah. O.K. It would be nice if this patch was marked as RFC (well, lack
> of signoff hints at this), or as WIP, or as PoC,...
Damn, I always forget about that.
>>>> The second patch exposes it from commitdiff view (obviosly), but also
>>>> from shortlog view, when less than 16 patches are begin shown.
>>>
>>> Why this nonconfigurable limit?
>>
>> Because the patch was actually a quick hack for the proof of concept
>> 8-) I wasn't even sure the patch idea would have been worth it (as
>> opposed to email-izing commitdiff_plain).
>
> Ah.
>
> Well, we might want to impose some limit to avoid generating and sending
> patchset for a whole history. Perhaps to page size (100), or some similar
> number?
The reason why I chose 16 is that (1) it's a rather commonly used
'small' number across gitweb and (2) it's a rather acceptable
'universal' upper limit for patchsets. There _are_ a few patchbombs
that considerably overtake that limit, but observe that this limit is
not an arbitrary limit on patchsets generated by the 'patchset' view,
but only a condition for which a link is generated from shortlog view.
We may want to have TWO limits here: one is the absolute maximum limit
to the number of patches dumped in a patchset (to prevent DoS attacks
by repeated requests of the whole history), and the other one is the
limit for autogenerated patchset links.
BTW, autogenerated patchset links probably make sense taking some
previous branch name as point of reference. e.g., if I have branch1
within the history of branch2, we probably want some (semi)automatic
way of getting a patchset for branch1..branch2 --of course, we also
want to do a shortlog between them, so that's a more general feature
we should think about.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: [PATCH 0/5] Detecting HEAD more reliably while cloning
From: Junio C Hamano @ 2008-12-01 2:54 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <7vabbhttq0.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> You may have noticed that the new git-send-email reversed the order of six
> patch files (one cover and five patches) I gave from the command line.
> Please consider this series as a bug report ;-)
>
> I think the bug is that "pop @ARGV" should read "shift @ARGV" or something
> silly and trivial like that, but it is getting late, so I won't debug
> tonight.
Perhaps this is a good enough fix? Very lightly tested.
-- >8 --
send-email: do not reverse the command line arguments
The loop picks elements from @ARGV one by one, sifts them into arguments
meant for format-patch and the script itself, and pushes them to @files
and @rev_list_opts arrays. Pick elements from @ARGV starting at the
beginning using shift, instead of at the end using pop, as push appends
them to the end of the array.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-send-email.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git c/git-send-email.perl w/git-send-email.perl
index 7508f8f..45beb9c 100755
--- c/git-send-email.perl
+++ w/git-send-email.perl
@@ -421,7 +421,7 @@ EOF
# Now that all the defaults are set, process the rest of the command line
# arguments and collect up the files that need to be processed.
my @rev_list_opts;
-while (my $f = pop @ARGV) {
+while (defined(my $f = shift @ARGV)) {
if ($f eq "--") {
push @rev_list_opts, "--", @ARGV;
@ARGV = ();
^ permalink raw reply related
* git-svn rebase "problems"
From: Joe Fiorini @ 2008-12-01 3:17 UTC (permalink / raw)
To: git
I'm having some problems with git svn rebase. I'm pretty sure this is
just the way git works, not a problem per se. But it's causing trouble
for me and my team.
My team is currently on Subversion. I'm trying to convince some people
that git is a good way to go.
So I'm using git-svn. My team tends to commit to the svn server fairly
often. It has happened more than once that, because git svn rebase
applies each svn commit sequentially, some of the commits will
conflict with each other - whether or not I have ever touched the
file. Obviously, this is a big problem because if I've never touched
the file, then I probably won't know exactly how to resolve the merge
(the merge markers haven't been solely reliable).
Is there anything I could do to get around this without having to
merge code I'm unfamiliar with?
Thanks all!
Joe Fiorini
--
joe fiorini
http://www.faithfulgeek.org
// freelancing & knowledge sharing
^ permalink raw reply
* two questions about the format of loose object
From: Liu Yubao @ 2008-12-01 8:00 UTC (permalink / raw)
To: git list
Hi,
In current implementation the loose objects are compressed:
loose object = deflate(typename + <space> + size + '\0' + data)
In sha1_file.c:unpack_sha1_file():
1) unpack_sha1_header() inflates first 8KB
2) parse_sha1_header() gets object's size
3) unpack_sha1_reset() allocates a (1+size) bytes buffer and
copy the first 8KB without header to it.
* Question 1:
Why not use the format below for loose object?
loose object = typename + <space> + size + '\0' + deflate(data)
So the size of loose object can be known before inflating it, in
step 3 above the 8KB memcpy isn't required.
In general, deflate() can decrease file size by 70% for text file,
I checked the git source and linux-2.6 source and got the statistical
data below:
.------------------+--------------+--------.
| | <= (8/0.3)KB | <= 8KB |
|------------------+--------------+--------|
| git-1.6.03 | 97% | 84% |
| linux-2.6.27-rc6 | 90% | 66% |
`------------------+--------------+--------'
* Question 2:
Why not use uncompressed loose object? That's to say:
loose object = typename + <space> + size + '\0' + data
I did a simple benchmark on my notebook and a server in my company,
writing a big file to disk is faster than compressing it first and
writing the result out. The former's performance for reading should
also be better because of file cache.
The current implementation caches objects in one process, the objects
can't be shared by many processes because they are uncompressed
to heap memory area of each process.
Uncompressed loose objects are better for sharing objects among
multiple git processes because they can be used directly after being
mmap-ed.
And I guess the most frequently used objects are loose objects
when you do some coding(git add, git diff, git diff --cached, git merge),
using uncompressed loose objects avoids uncompressing loose objects again
and again.
Below is the result of my simple benchmark:
########################################
# on my notebook
$ perl b.pl git-1.5.6/Makefile 1000
Rate compressed uncompressed
compressed 198/s -- -92%
uncompressed 2463/s 1147% --
$ perl b.pl git-1.5.6/parse-options.c 2000
Rate compressed uncompressed
compressed 341/s -- -88%
uncompressed 2845/s 734% --
$ find git-1.5.6/ -name "*.[ch]" -exec cat {} + > all.c
$ perl b.pl all.c 1000
Rate compressed uncompressed
compressed 3.39/s -- -97%
uncompressed 111/s 3182% --
#######################################
# on a server
$ perl b.pl Makefile 6000
(warning: too few iterations for a reliable count)
Rate compressed uncompressed
compressed 447/s -- -98%
uncompressed 18750/s 4094% --
$ perl b.pl parse-options.c 8000
(warning: too few iterations for a reliable count)
Rate compressed uncompressed
compressed 1130/s -- -97%
uncompressed 33333/s 2850% --
$ perl b.pl all.c 1000
Rate compressed uncompressed
compressed 5.48/s -- -95%
uncompressed 115/s 1997%
#####################################################
# b.pl
#!/usr/bin/perl
use strict;
use warnings;
use Benchmark qw(:hireswallclock cmpthese);
use File::Slurp;
use IO::Compress::Deflate qw(deflate $DeflateError);
my $text = read_file($ARGV[0], binmode => ':raw');
cmpthese($ARGV[1], {'compressed' => \&zip, 'uncompressed' => \&output});
sub zip {
deflate \$text => 'all.c.z' || die "$!\n";
}
sub output {
write_file("all2.c", {binmode => ':raw'}, $text);
}
^ permalink raw reply
* Re: how to hide some branches
From: Peter Krefting @ 2008-12-01 8:01 UTC (permalink / raw)
To: Pascal Obry; +Cc: git list
In-Reply-To: <493261C9.4040608@obry.net>
Pascal Obry:
> I'd like to hide some (not removing them).
> Is there a solution for this?
git branch --no-merged
shows only the branches that has not been merged into your current
working branch. That might be one solution.
I don't know if it is possible to set it as a default option, but I
would also appreciate such a feature.
I use the "git branch --merged" to move away merged branches into a
different "namespace" (branch "a" becomes "merged/a"):
for branch in $(git branch --merged master | egrep -v 'merged|master');
do
git branch -m "$branch" "merged/$branch"
done
--
\\// Peter - http://www.softwolves.pp.se/
^ permalink raw reply
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