Git development
 help / color / mirror / Atom feed
* Re: [PATCH 2/3] add new Git::Repo API
From: Petr Baudis @ 2008-07-18 18:19 UTC (permalink / raw)
  To: Lea Wiemann; +Cc: Jakub Narebski, git, John Hawley
In-Reply-To: <4880DC6C.7090708@gmail.com>

On Fri, Jul 18, 2008 at 08:09:48PM +0200, Lea Wiemann wrote:
> Also, gitweb isn't using cmd_output because it needs a pipe interface,
> but because it needs a caching layer in between -- most applications
> would do just fine with open calls.

One of the points of the API is to abstract these out.

> > As I said, majority of Git API usage is actually the pipe API. So we
> > should figure out how to provide it. I agree that it's not immediately
> > within your scope, but you are introducing new Perl API and this just
> > needs to be embedded somewhere there consistently.
> 
> Sure, but pleeeease not as part of this patch series! :-)  Look, our
> conversation is going something like this:
> 
> Lea: Here's a Perl API that fell out of my gitweb development for free.
> Petr: I want a pony with the API!
> Lea: But I don't have a pony.  Can we please just go with the Perl API
> as a start, even if I don't supply ponies with it?
> 
> (Cf. the very cute <http://c2.com/cgi/wiki?IwantaPony>.)

I'm fine with that, as long as the version that enters into master will
have a pony so that we stay with a single pony within the codebase in
the end, not two ponies with differently shaped saddles.

But as I said, I'm going to work on that.

> >> If you're getting a SHA1 through the user-interface, check its existence
> >> with get_sha1 before passing it to the constructor.
> > 
> > But that's an expensive operation, you need extra Git exec for this,
> 
> For the gazillionth time in this thread, there is no extra exec.  It's a
> write to a bidirectional cat-file --batch-check pipe.  It's not
> expensive.  Really. ;-)

But the API is still obnoxiously elaborate, as I complained in another
mail.

> >> I have resolving code in gitweb's git_get_sha1_or_die
> > 
> > The thing that concerns me about this is that this might show that your
> > approach to error handling is not flexible enough for some real-world
> > usage and this might be a design mistake - is that not so?
> 
> I don't think so; the error handling is fine.  Given that I want
> fine-granular error reporting for gitweb, there *needs* to be a
> git_get_sha1_or_die function; you can't move that into the API.

Wait, this doesn't compute here. The error handling is fine, but it is
actually not fine for gitweb. Can't we make it fine for everyone?

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* Re: [PATCH] api-run-command.txt: typofix
From: Johannes Sixt @ 2008-07-18 18:12 UTC (permalink / raw)
  To: Stephan Beyer; +Cc: git, Junio C Hamano
In-Reply-To: <1216338835-10627-1-git-send-email-s-beyer@gmx.net>

On Freitag, 18. Juli 2008, Stephan Beyer wrote:
> -`run_command_v_opt`, `run_command_v_opt_dir`, `run_command_v_opt_cd_env`::
> +`run_command_v_opt`, `run_command_v_opt_cd`, `run_command_v_opt_cd_env`::

Correct! Thanks.

-- Hannes

^ permalink raw reply

* Re: [PATCH 2/3] add new Git::Repo API
From: Lea Wiemann @ 2008-07-18 18:09 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Jakub Narebski, git, John Hawley
In-Reply-To: <20080718164828.GT10151@machine.or.cz>

Petr Baudis wrote:
> [$repo->_cmd_output:]
> 
> we _need_ such a wrapper _publically_, because it tends to be
> actually the main use-case of Git.pm,

Well, sure, I happen to not be convinced, but it *may* be useful.  The
point I'm trying to make is that it's not part of what I'm writing here.

> as part of your gitweb migration to Git::Repo, you will temporarily
> introduce calls to _cmd_output(), the "internal" API. :-) Sure, it's
> only temporary, but many won't have the luxury to adjust the Git::Repo
> API to provide all the operations they need, and ultimately they will
> need to defer to the pipe interface.

Yup, and I'm actually fine with that.  (I'll probably alias _cmd_output
to cmd_output in gitweb, just to make it clear that it is, for the
purpose of gitweb, a *supported* mode of operation.)  If the
Git::Repo::_cmd_output API goes away, you'll have to insert a few lines
of code in gitweb, but that's it.  Really, no big deal.

Also, gitweb isn't using cmd_output because it needs a pipe interface,
but because it needs a caching layer in between -- most applications
would do just fine with open calls.

> As I said, majority of Git API usage is actually the pipe API. So we
> should figure out how to provide it. I agree that it's not immediately
> within your scope, but you are introducing new Perl API and this just
> needs to be embedded somewhere there consistently.

Sure, but pleeeease not as part of this patch series! :-)  Look, our
conversation is going something like this:

Lea: Here's a Perl API that fell out of my gitweb development for free.
Petr: I want a pony with the API!
Lea: But I don't have a pony.  Can we please just go with the Perl API
as a start, even if I don't supply ponies with it?

(Cf. the very cute <http://c2.com/cgi/wiki?IwantaPony>.)

>> If you're getting a SHA1 through the user-interface, check its existence
>> with get_sha1 before passing it to the constructor.
> 
> But that's an expensive operation, you need extra Git exec for this,

For the gazillionth time in this thread, there is no extra exec.  It's a
write to a bidirectional cat-file --batch-check pipe.  It's not
expensive.  Really. ;-)

>> I have resolving code in gitweb's git_get_sha1_or_die
> 
> The thing that concerns me about this is that this might show that your
> approach to error handling is not flexible enough for some real-world
> usage and this might be a design mistake - is that not so?

I don't think so; the error handling is fine.  Given that I want
fine-granular error reporting for gitweb, there *needs* to be a
git_get_sha1_or_die function; you can't move that into the API.

-- Lea

^ permalink raw reply

* Re: [StGit PATCH] Test that we can add a new file to a non-topmost patch with refresh -p
From: Jon Smirl @ 2008-07-18 17:45 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: Catalin Marinas, git
In-Reply-To: <20080718180109.GA14825@diana.vm.bytemark.co.uk>

On 7/18/08, Karl Hasselström <kha@treskal.com> wrote:
> On 2008-07-18 19:03:06 +0200, Karl Hasselström wrote:
>
>  > We currently can't -- this is bug 12038, found by Jon Smirl. See
>  >
>  >   https://gna.org/bugs/index.php?12038
>
>  OK, the problem is that to pop the patches on top of the one we are to
>  refresh, we call pop_patch(keep = True). This in turn calls
>  git.switch(keep = True), which resets the index (but is careful to not
>  touch the worktree).
>
>  I'm not quite sure how to fix this in a simple way -- the code simply
>  assumes that the index contains nothing of interest. And since I
>  already have a rewrite of refresh that handles this and a handful of
>  other cases that the old code does not, I'm kind of disinclined to
>  undertake a larger restructuring of the code.

It's no big deal to me, it is easy to work around. But it did take me
a while to notice that the add was missing.

When is the refresh rewrite going to be ready for prime time?

>
>  Catalin, what do you think?
>
>
>  --
>  Karl Hasselström, kha@treskal.com
>       www.treskal.com/kalle
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [StGit PATCH] Test that we can add a new file to a non-topmost patch with refresh -p
From: Karl Hasselström @ 2008-07-18 18:01 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git, Jon Smirl
In-Reply-To: <20080718170225.10086.17504.stgit@yoghurt>

On 2008-07-18 19:03:06 +0200, Karl Hasselström wrote:

> We currently can't -- this is bug 12038, found by Jon Smirl. See
>
>   https://gna.org/bugs/index.php?12038

OK, the problem is that to pop the patches on top of the one we are to
refresh, we call pop_patch(keep = True). This in turn calls
git.switch(keep = True), which resets the index (but is careful to not
touch the worktree).

I'm not quite sure how to fix this in a simple way -- the code simply
assumes that the index contains nothing of interest. And since I
already have a rewrite of refresh that handles this and a handful of
other cases that the old code does not, I'm kind of disinclined to
undertake a larger restructuring of the code.

Catalin, what do you think?

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* Re: [RFC PATCH] Support gitlinks in fast-import/export.
From: Johannes Schindelin @ 2008-07-18 17:36 UTC (permalink / raw)
  To: Alexander Gavrilov; +Cc: git
In-Reply-To: <200807182103.37272.angavrilov@gmail.com>

Hi,

On Fri, 18 Jul 2008, Alexander Gavrilov wrote:

> 	What I'm unsure of is, should fast-export try to reuse commit
> 	marks for gitlinks where it happened to recognize the object,
> 	or always output the SHA as it is stored in the tree?

Are they commit marks?  No.  So they should be handled as marks, just as 
those for blobs and trees.

(They are commit marks in the _submodule_, but that does not matter here.)

> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
> index 395c055..80c591a 100644
> --- a/Documentation/git-fast-import.txt
> +++ b/Documentation/git-fast-import.txt
> @@ -481,6 +481,9 @@ in octal.  Git only supports the following modes:
>    what you want.
>  * `100755` or `755`: A normal, but executable, file.
>  * `120000`: A symlink, the content of the file will be the link target.
> +* `160000`: A gitlink, SHA-1 of the object refers to a commit in
> +  another repository. Git links can only be specified by SHA or through
> +  a commit mark. They are used to implements submodules.

s/\(implement\)s/\1/

> diff --git a/builtin-fast-export.c b/builtin-fast-export.c
> index d0a462f..14b1549 100644
> --- a/builtin-fast-export.c
> +++ b/builtin-fast-export.c
> @@ -123,8 +123,19 @@ static void show_filemodify(struct diff_queue_struct *q,
>  			printf("D %s\n", spec->path);
>  		else {
>  			struct object *object = lookup_object(spec->sha1);
> -			printf("M %06o :%d %s\n", spec->mode,
> -			       get_object_mark(object), spec->path);
> +			int mark = object ? get_object_mark(object) : 0;

As I said, that looks wrong.  Maybe we have to fake objects for the 
gitlinks.

> @@ -183,7 +194,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
>  				    "", &rev->diffopt);
>  
>  	for (i = 0; i < diff_queued_diff.nr; i++)
> -		handle_object(diff_queued_diff.queue[i]->two->sha1);
> +		if (!S_ISGITLINK(diff_queued_diff.queue[i]->two->mode))
> +			handle_object(diff_queued_diff.queue[i]->two->sha1);

Why?  You do not want to export changes in the submodules?

> diff --git a/fast-import.c b/fast-import.c
> index e72b286..e7977c1 100644
> --- a/fast-import.c
> +++ b/fast-import.c

I'll let Shawn comment on that.  Oh, wait, it's his last day in work 
today.  Maybe I have something useful to say about this part of the patch, 
then, to save Shawn some work.

> @@ -1900,7 +1901,16 @@ static void file_change_m(struct branch *b)
>  		p = uq.buf;
>  	}
>  
> -	if (inline_data) {
> +	if (S_ISGITLINK(mode)) {
> +		if (inline_data)
> +			die("Git links cannot be specified 'inline': %s",
> +				command_buf.buf);
> +		else if (oe) {
> +			if (oe->type != OBJ_COMMIT)
> +				die("Not a commit (actually a %s): %s",
> +					typename(oe->type), command_buf.buf);

How is that supposed to work?  Do I understand correctly that you require 
the user to construct a commit object for the gitlink?  That would be 
actively wrong.

Oh, and your patch lacks test cases that demonstrate how you envisage the 
whole thing to work.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 2/3] add new Git::Repo API
From: Petr Baudis @ 2008-07-18 17:17 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Lea Wiemann, git, John Hawley
In-Reply-To: <200807181905.07123.jnareb@gmail.com>

On Fri, Jul 18, 2008 at 07:05:05PM +0200, Jakub Narebski wrote:
> Dnia piątek 18. lipca 2008 18:48, Petr Baudis napisał:
> > On Tue, Jul 15, 2008 at 12:19:31AM +0200, Lea Wiemann wrote:
> 
> > > No, you should never pass in an invalid SHA1 in the first place.  The
> > > above piece of documentation is just a warning that bugs will show up
> > > delayed.  IOW, this is not the right place to have your error handling.
> > > 
> > > If you're getting a SHA1 through the user-interface, check its existence
> > > with get_sha1 before passing it to the constructor.
> > 
> > But that's an expensive operation, you need extra Git exec for this,
> > while all the Git commands can do the checks for you, if you give them
> > the chance.
> > 
> > I was doing pretty much this thing in Cogito (initially out of
> > necessity) and it made it ungodly slow for any kind of batch operations.
> 
> Lea probably would point out that thanks to '--batch-check' option
> to git-cat-file, and "reuse => 1" option to ->get_bidi_pipe you would
> need only one extra Git exec...  BUT it is one extra Git exec per
> entire Perl script (one per request, for example, for gitweb).
> And it wouldn't help batching Perl script commands.

Even so, I don't like this restriction simply since it makes the usage
more complicated - in my commandline tool, I will be forced to write

	my $csha1 = $repo->get_sha1($ARGV[0]);
	$csha1 or die "a random inconsistent error message";
	$repo->get_commit($csha1)

instead of

	$repo->get_commit($ARGV[0]) # die if unresolved

and I don't understand why. Me and 80% of the scripts don't *care* about
some more graceful error handling, and if gitweb and the other 20%
(or less) do, *they* should do the extra work, not me.

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* Re: copy selected history between repostories
From: Avery Pennarun @ 2008-07-18 17:12 UTC (permalink / raw)
  To: luisgutz; +Cc: git
In-Reply-To: <18533605.post@talk.nabble.com>

On 7/18/08, luisgutz <luis@xmos.com> wrote:
>  Is there any way to import that directory into repoA with all it's history,
>  but NOT the history from the other commits?
>  Another way of putting is this: can I make git forget the history of all
>  other commits but those from this directory?

Try cloning the repository, then running git-filter-branch with the
--subdirectory-filter option.  It works quite nicely.

Have fun,

Avery

^ permalink raw reply

* Re: [PATCH 2/3] add new Git::Repo API
From: Jakub Narebski @ 2008-07-18 17:05 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Lea Wiemann, git, John Hawley
In-Reply-To: <20080718164828.GT10151@machine.or.cz>

Dnia piątek 18. lipca 2008 18:48, Petr Baudis napisał:
> On Tue, Jul 15, 2008 at 12:19:31AM +0200, Lea Wiemann wrote:

> > No, you should never pass in an invalid SHA1 in the first place.  The
> > above piece of documentation is just a warning that bugs will show up
> > delayed.  IOW, this is not the right place to have your error handling.
> > 
> > If you're getting a SHA1 through the user-interface, check its existence
> > with get_sha1 before passing it to the constructor.
> 
> But that's an expensive operation, you need extra Git exec for this,
> while all the Git commands can do the checks for you, if you give them
> the chance.
> 
> I was doing pretty much this thing in Cogito (initially out of
> necessity) and it made it ungodly slow for any kind of batch operations.

Lea probably would point out that thanks to '--batch-check' option
to git-cat-file, and "reuse => 1" option to ->get_bidi_pipe you would
need only one extra Git exec...  BUT it is one extra Git exec per
entire Perl script (one per request, for example, for gitweb).
And it wouldn't help batching Perl script commands.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* [RFC PATCH] Support gitlinks in fast-import/export.
From: Alexander Gavrilov @ 2008-07-18 17:03 UTC (permalink / raw)
  To: git

Currently fast-import/export cannot be used for
repositories with submodules. This patch extends
the relevant programs to make them correctly
process gitlinks.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---

	I noticed that fast-export & fast-import cannot work with
	repositories using submodules: import complains about
	an invalid mode, and export fails while trying to open the SHA
	as a blob.

	As I didn't see any particular reason for it to be so, I tried to
	implement support for gitlinks.

	What I'm unsure of is, should fast-export try to reuse commit
	marks for gitlinks where it happened to recognize the object,
	or always output the SHA as it is stored in the tree?

	-- Alexander

 Documentation/git-fast-import.txt |    3 +++
 builtin-fast-export.c             |   18 +++++++++++++++---
 fast-import.c                     |   12 +++++++++++-
 3 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 395c055..80c591a 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -481,6 +481,9 @@ in octal.  Git only supports the following modes:
   what you want.
 * `100755` or `755`: A normal, but executable, file.
 * `120000`: A symlink, the content of the file will be the link target.
+* `160000`: A gitlink, SHA-1 of the object refers to a commit in
+  another repository. Git links can only be specified by SHA or through
+  a commit mark. They are used to implements submodules.
 
 In both formats `<path>` is the complete path of the file to be added
 (if not already existing) or modified (if already existing).
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index d0a462f..14b1549 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -123,8 +123,19 @@ static void show_filemodify(struct diff_queue_struct *q,
 			printf("D %s\n", spec->path);
 		else {
 			struct object *object = lookup_object(spec->sha1);
-			printf("M %06o :%d %s\n", spec->mode,
-			       get_object_mark(object), spec->path);
+			int mark = object ? get_object_mark(object) : 0;
+
+			if (mark)
+				printf("M %06o :%d %s\n", spec->mode,
+				       mark, spec->path);
+			else {
+				if (!S_ISGITLINK(spec->mode))
+					die("Unknown object added: %s at %s",
+					    sha1_to_hex(spec->sha1), spec->path);
+
+				printf("M %06o %s %s\n", spec->mode,
+				       sha1_to_hex(spec->sha1), spec->path);
+			}
 		}
 	}
 }
@@ -183,7 +194,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
 				    "", &rev->diffopt);
 
 	for (i = 0; i < diff_queued_diff.nr; i++)
-		handle_object(diff_queued_diff.queue[i]->two->sha1);
+		if (!S_ISGITLINK(diff_queued_diff.queue[i]->two->mode))
+			handle_object(diff_queued_diff.queue[i]->two->sha1);
 
 	mark_object(&commit->object);
 	if (!is_encoding_utf8(encoding))
diff --git a/fast-import.c b/fast-import.c
index e72b286..e7977c1 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1868,6 +1868,7 @@ static void file_change_m(struct branch *b)
 	case S_IFREG | 0644:
 	case S_IFREG | 0755:
 	case S_IFLNK:
+	case S_IFGITLINK:
 	case 0644:
 	case 0755:
 		/* ok */
@@ -1900,7 +1901,16 @@ static void file_change_m(struct branch *b)
 		p = uq.buf;
 	}
 
-	if (inline_data) {
+	if (S_ISGITLINK(mode)) {
+		if (inline_data)
+			die("Git links cannot be specified 'inline': %s",
+				command_buf.buf);
+		else if (oe) {
+			if (oe->type != OBJ_COMMIT)
+				die("Not a commit (actually a %s): %s",
+					typename(oe->type), command_buf.buf);
+		}
+	} else if (inline_data) {
 		static struct strbuf buf = STRBUF_INIT;
 
 		if (p != uq.buf) {
-- 
1.5.6.3.18.gfe82

^ permalink raw reply related

* [StGit PATCH] Test that we can add a new file to a non-topmost patch with refresh -p
From: Karl Hasselström @ 2008-07-18 17:03 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git, Jon Smirl
In-Reply-To: <20080718084127.GA7042@diana.vm.bytemark.co.uk>

We currently can't -- this is bug 12038, found by Jon Smirl. See

  https://gna.org/bugs/index.php?12038

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

Here's a proper test that demonstrates the bug. It applies to the
stable branch.

 t/t2701-refresh-p.sh |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 46 insertions(+), 0 deletions(-)
 create mode 100755 t/t2701-refresh-p.sh


diff --git a/t/t2701-refresh-p.sh b/t/t2701-refresh-p.sh
new file mode 100755
index 0000000..d42e90f
--- /dev/null
+++ b/t/t2701-refresh-p.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+
+test_description='Run "stg refresh -p"'
+
+. ./test-lib.sh
+
+# Ignore our own temp files.
+cat >> .git/info/exclude <<EOF
+expected*.txt
+files*.txt
+status*.txt
+EOF
+
+test_expect_success 'Initialize StGit stack' '
+    stg init &&
+    for i in 1 2; do
+        echo x > $i.txt &&
+        git add $i.txt &&
+        stg new p$i -m "Patch $i" &&
+        stg refresh
+    done
+'
+
+touch expected0.txt
+cat > expected1.txt <<EOF
+A 1.txt
+A new.txt
+EOF
+cat > expected2.txt <<EOF
+A 2.txt
+EOF
+test_expect_failure 'Add new file to non-top patch' '
+    stg status > status1.txt &&
+    diff -u expected0.txt status1.txt &&
+    echo y > new.txt &&
+    git add new.txt &&
+    stg refresh -p p1 &&
+    stg status > status2.txt &&
+    diff -u expected0.txt status2.txt &&
+    stg files p1 > files1.txt &&
+    diff -u expected1.txt files1.txt &&
+    stg files p2 > files2.txt &&
+    diff -u expected2.txt files2.txt
+'
+
+test_done

^ permalink raw reply related

* Re: Considering teaching plumbing to users harmful
From: Ping Yin @ 2008-07-18 17:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Avery Pennarun, Johannes Schindelin, git
In-Reply-To: <7vr69tpoze.fsf@gitster.siamese.dyndns.org>

On Thu, Jul 17, 2008 at 4:12 AM, Junio C Hamano <gitster@pobox.com> wrote:

> Suppose you are about to finish something you have been cooking (say a
> series of five logical commits), you've made three of these commits
> already, and what you have in your work tree and the index is to be split
> into the last two commits.  Somehow you learn that $x above has a updated
> version.
>
> Yes, running "git stash && git pull --rebase && git stash pop" would be
> better than running "git pull --rebase" alone from that state.  But that
> would mean your history would have your first 3 commits (of 5 commit
> series), somebody else's totally unrelated commits, and then you will work
> on finishing the remaining 2 commits on top of it.

Hmm, the first 3 commits are not pushed out, right? So by "rebase",
the history should be first the somebody else's commits
(origin/master), then the first 3 commits, then the remaining 2
commits?:


-- 
Ping Yin

^ permalink raw reply

* copy selected history between repostories
From: luisgutz @ 2008-07-18 16:58 UTC (permalink / raw)
  To: git


Hi All,

This is something I'm not sure how to do with git, or even if it is possible
in git; and because google has not been my friend this time, I though I
would ask here.

I have 2 repositories: repoA and repoB.

repoB is massive. lots of history and files. But it has a set of files
inside a particular dir with 40-60 files that I just realized are better in
repoA.

Is there any way to import that directory into repoA with all it's history,
but NOT the history from the other commits?
Another way of putting is this: can I make git forget the history of all
other commits but those from this directory?


-- 
View this message in context: http://www.nabble.com/copy-selected-history-between-repostories-tp18533605p18533605.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* Re: [PATCH 2/3] add new Git::Repo API
From: Petr Baudis @ 2008-07-18 16:54 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Lea Wiemann, git, John Hawley
In-Reply-To: <200807150141.39186.jnareb@gmail.com>

On Tue, Jul 15, 2008 at 01:41:38AM +0200, Jakub Narebski wrote:
> On Mon, 14 July 2008, Petr Baudis wrote:
> > Here is an idea: Introduce Git::Command object that will have very
> > general interface and look like
> > 
> > 	my $c = Git::Command->new(['git', '--git-dir=.', 'cat-file', \
> > 		'-p', 'bla'], {pipe_out=>1})
> > 	...
> > 	$c->close();
> 
> Errr... how do you read from such a pipe?  <$c> I think wouldn't work,
> unless you would use some trickery...

That's good point; it might either be done using some trickery, or
$c->pipe. The idea behind having a special object for it though is to
have *unified* (no matter how simple) error handling. You might not
detect the command erroring out at the open time.

Is there a better approach for solving this?

> > and a Git::CommandFactory with a nicer interface that would look like
> > 
> > 	my $cf = Git::CommandFactory->new('git', '--git-dir=.');
> > 	my $c = $cf->output_pipe('cat-file', '-p', 'bla');
> > 	$c->close();
> > 
> > Then, Git::Repo would have a single Git::CommandFactory instance
> > pre-initialized with the required calling convention, and returned by
> > e.g. cmd() method. Then, from the user POV, you would just:
> > 
> > 	my $repo = Git::Repo->new;
> > 	$repo->cmd->output_pipe('cat-file', '-p', 'bla');
> > 
> > Or am I overdoing it?
> 
> You are probably overdoing it.
> 
> 
> I think it would be good to have the following interface
> 
> Git->output_pipe('ls-remotes', $URL, '--heads');

This is problematic; I think mixing the new and old interface within a
single class is very bad idea, we should have Git::Standalone or
something for this. Or, just, default Git::CommandFactory. ;-)

> [...]
> $r = Git::Repo->new(<git_dir>);
> $r->output_pipe('ls_tree', 'HEAD');
> [...]
> $nb = Git::Repo::NonBare->new(<git_dir>[, <working_area>]);
> $nb->output_pipe('ls-files');
> 
> 
> How can it be done with minimal effort, unfortunately I don't know...

Well, this interface is almost identical to what I delineated, except
that I have the extra ->cmd-> step there. But maybe, we could go with
your API and instead have Git::CommandFactory as a base of Git::Repo?
The hierarchy would be

	Git::CommandFactory - provides the cmd_pipe toolkit
		|
	    Git::Repo       - provides repository model
		|
	Git::Repo::NonBare  - additional working-copy-related methods

I think I will post a sample implementation sometime over the weekend.

> > Another thing is clearly describing how error handling is going to work.
> > I have not much against ditching Error.pm, but just saying "die + eval"
> > does not cut it - how about possible sideband data? E.g. the failure
> > mode of Git.pm's command() method includes passing the error'd command
> > output in the exception object. How are we going to handle it? Now, it
> > might be actually okay to say that we _aren't_ going to handle this if
> > it is deemed unuseful, but that needs to be determined too. I don't know
> > off the top of my head.
> 
> I think that the solution might be some output_pipe option on how to
> treat command exit status, command STDERR, and errors when invoking
> command (for example command not found).
> 
> Mentioned http://http://www.perl.com/pub/a/2002/11/14/exception.html
> explains why one might want to use Error.pm.

The arguments against its usage that popped up over the year(s?):

	(i) It is not standard practice in the Perl world

	(ii) It is syntactically ambiguous, c.f. Lea's report about
	the missing semicolon

	(iii) The usage of closures in this way has inherent memory leak
	issues

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* Re: [PATCH 2/3] add new Git::Repo API
From: Lea Wiemann @ 2008-07-18 16:51 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, John Hawley, Petr Baudis
In-Reply-To: <200807181735.15278.jnareb@gmail.com>

Jakub Narebski wrote:
> Besides if decoding is done in Perl API, we can convert it simply
> to Perl internal form (which, IIUC, in modern Perl is UTF-8 and
> marked as such).

FWIW, Perl strings actually contain non-encoded Unicode code points.
IOW, they're not byte strings.

> Still I think that putting cmd_output and other in Git::Repo
> is not a good API.

Yup; that's why I'm underscore-prefixing it (and taking it out of the
man page) for the next version of this patch.

> By the way, would you prefer if I commented on 3/3 patch as it is now,
> [...] or would you rather I wait for next round (next version) of patches?

I suggest you wait for the next version of the patch series, which I'll
post in a few hours.  There are quite a few changes; I'll list them in
the parent message for the series.

-- Lea

^ permalink raw reply

* Re: [PATCH 2/3] add new Git::Repo API
From: Petr Baudis @ 2008-07-18 16:48 UTC (permalink / raw)
  To: Lea Wiemann, Jakub Narebski; +Cc: git, John Hawley
In-Reply-To: <487BD0F3.2060508@gmail.com>

In order to keep this mail within sensible size, I have trimmed some of
the bits - basically, the default reply is usually ranging from "Fair
enough" to "Great!" :-)

On Tue, Jul 15, 2008 at 12:19:31AM +0200, Lea Wiemann wrote:
> > First, I don't think it's good idea at all to put the pipe-related stuff
> > to Git::Repo - this is botched up API just like the current one.
> 
> Well, they're more like helper methods.  Since they don't fit into the
> design goals of the Git::Repo API at all, I'd suggest we just
> underscore-prefix them and take them out of the man page.  (The only
> reason why I hadn't done this is that gitweb uses $repo->cmd_output
> extensively, so it'd end up with a lot of underscore calls.  But I
> suppose we can either alias _cmd_output to cmd_output in gitweb's
> CachedRepo subclass, or live with $repo->_cmd_output calls.)  Does
> underscore-prefixing sound good to you?

I think this is one problematic point we keep hitting - my opinion is
that we _need_ such a wrapper _publically_, because it tends to be
actually the main use-case of Git.pm, and that this wrapper should be:

	(i) both available standalone for commands like ls-remote

	(ii) and available as part of Git::Repo instance, to have the
	right arguments passed to Git automagically

So you define cmd_output() and seem to argue that this command should
not be called directly and is not interesting for the outside. But the
experience shows that the pipe interface is actually the _most_ used
part of the Git Perl API, and in fact you mentioned that as part of your
gitweb migration to Git::Repo, you will temporarily introduce calls to
_cmd_output(), the "internal" API. :-) Sure, it's only temporary, but
many won't have the luxury to adjust the Git::Repo API to provide all
the operations they need, and ultimately they will need to defer to the
pipe interface.

> > It should be actually very easy to start with moving all the pipe
> > functionality to Git::Command.
> 
> Creating a new (Git::Command) API is very much non-trivial, apart from
> the fact that I'm not convinced that we need Git::Command, and that a
> clean command interface neither falls out of Git.pm nor Git::Repo.

As I said, majority of Git API usage is actually the pipe API. So we
should figure out how to provide it. I agree that it's not immediately
within your scope, but you are introducing new Perl API and this just
needs to be embedded somewhere there consistently.

> >> [Git::Commit->new, Git::Tag->new:]
> >> +Calls to this method are free, since it does not check whether $sha1
> >> +exists and has the right type.  However, accessing any of the commit
> >> +object's properties will fail if $sha1 is not a valid commit object.
> > 
> > This is nice idea, but I'd also provide a well-defined way for the user
> > to verify the object's validity at a good moment; basically, make load()
> > a public method. The user can deal with errors then and rely on
> > error-free behavior later.
> 
> No, you should never pass in an invalid SHA1 in the first place.  The
> above piece of documentation is just a warning that bugs will show up
> delayed.  IOW, this is not the right place to have your error handling.
> 
> If you're getting a SHA1 through the user-interface, check its existence
> with get_sha1 before passing it to the constructor.

But that's an expensive operation, you need extra Git exec for this,
while all the Git commands can do the checks for you, if you give them
the chance.

I was doing pretty much this thing in Cogito (initially out of
necessity) and it made it ungodly slow for any kind of batch operations.

> >> +Note that $sha1 must be the SHA1 of a commit object; tag objects are
> >> +not dereferenced.
> > 
> > Why not?
> 
> Because the SHA1 might resolve to an object of the wrong type, which
> means you have to do error handling in Git::Object objects; that's the
> wrong place.
> 
> If tag-resolving is really needed, we can add an optional $type
> parameter to get_sha1, which will cause get_sha1 to resolve the object
> until a $type object is found, or return undef if the object is or
> resolves to an object of the wrong type.

See above why I think you should reconsider requiring the explicit
"resolving" step.

> I have resolving code in gitweb's git_get_sha1_or_die (which I didn't
> implement in Git::Repo since it uses some customized error reporting).
> The resolving code could conceivably be extracted and moved to get_sha1.
>  I think there are a few things to ponder and maybe discuss, so I'd do
> that in a separate patch (if I get around it before the end of the project).

The thing that concerns me about this is that this might show that your
approach to error handling is not flexible enough for some real-world
usage and this might be a design mistake - is that not so? I didn't look
at the code.

> >> [Snipped a lot of quoting --LW]
> >> +=item $repo->repo_dir
> >> +=item $repo->git_binary
> >> +=item $repo->version
> >> +sub _get_git_cmd {
> > 
> > This definitely does not belong to a Git::Repo object.
> 
> Which of those methods are you referring to?  I think $repo->version
> might reasonably be removed (and the code re-added to gitweb); I'll do
> so unless you object.  _get_git_cmd is already underscored, and repo_dir
> and git_binary only access attributes passed in through the constructor,
> so I think those three should stay.

Sorry, you're right about repo_dir and possibly git_binary. My main
concern was about the command pipe handling itself, but I elaborated on
that above already.

> >> +=item $repo->get_refs
> >> +=item $repo->get_refs($pattern)
> > 
> > Again, the refs should be properly integrated into the object structure.
> 
> Really?  I think it's generally fine for get_refs to exist and to live
> in Git::Repo.
> 
> Its return value (currently an an arrayref of [$sha1, $object_type,
> $ref_name] arrayrefs) might need improvement though, and I find the
> $pattern parameter pretty suspect (in that it smells like a for-each-ref
> wrapper).  Since get_refs is unused at the moment (gitweb ended up
> needing the slightly different show-ref), I'll remove it for now.  (Same
> thing about me not being a fan of premature API design applies.)

Just a note, the thing is that you might want to add some methods for
inspecting and mutating the refs, and at the same time this is not a
repository-specific concept, but you can get the same structure from
git ls-remote call. That's why I think it would make sense to make
a separate object out of it. But that's moot point now that the API
won't be there yet.

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* Re: git submodules and commit
From: Ping Yin @ 2008-07-18 16:11 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: Nigel Magnay, Git Mailing List
In-Reply-To: <32541b130807160843k25f1d7d3u8bfecd6c1c6eab91@mail.gmail.com>

On Wed, Jul 16, 2008 at 11:43 PM, Avery Pennarun <apenwarr@gmail.com> wrote:
> On 7/16/08, Nigel Magnay <nigel.magnay@gmail.com> wrote:

> If you want to work with me on my new submodule workflow (and I'd
> certainly appreciate it!) then I'd suggest one or more of the
> following starting points:
>
> - Take the recursive push, pull, and update operations described
> above, make them general (ie. not referring to my submodules by name
> :)), and add them as commands in the real git-submodule script.  The
> trickiest part here will be figuring out which remote branch to
> push/pull.

See http://article.gmane.org/gmane.comp.version-control.git/69834
([PATCH] Added recurse command to git submodule)
Or search "submodule recursive" in gmane.

The recursive pull,diff,status for submodule is implemented by Imran M
Yousuf. And IIRC, with this patch, you can walk through the submodule
hierarchy to exectute any command.




-- 
Ping Yin

^ permalink raw reply

* Re: [PATCH] Teach git submodule update to use distributed repositories
From: Petr Baudis @ 2008-07-18 15:49 UTC (permalink / raw)
  To: Nigel Magnay; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <320075ff0807180809x599aefafw2c7fe88fea2691d2@mail.gmail.com>

On Fri, Jul 18, 2008 at 04:09:40PM +0100, Nigel Magnay wrote:
> Hmm. Locally modifying my .gitmodules still feels bad because I don't
> like either of those tradeoffs (but I don't have any sensible
> suggestion yet).
> 
> As a bit of background (as to why I'd dislike (a) and (b)), we had a
> team switch to git, and one of the really nice things is the ability
> to share stuff around and branch freely - but the flipside of that is
> that we tend to push to a central repo more rarely, so the advantages
> of an continuous integration server become less. What we did is to
> tell a centralised CI server the URLs of all the team's git
> repositories, and it would periodically pull from them, speculatively
> compile anything new, and run the big suite of tests - finishing up by
> emailling them a heads-up that a particular state in their repo is
> 'bad'.
> 
> This was really popular as it was demonstrably better than anything we
> could do with svn, and best of all, it's pretty much transparent - as
> a user you don't have to do anything at all.
> 
> I could do it now by hacking about with files; it'd just be nice to
> keep it transparent and make it a directly supported feature.

In that case you would need the "URL mappings", perhaps as a per-remote
attribute. That is, you could configure:

	"When I am doing git pull fred, do git submodule update but
	apply remote.fred.subrewrite sed script on each URL before
	fetching the submodule."

Still, that feels quite hackish to me, and I'm not convinced that your
workflow cannot be adjusted so that users merge only the next-to-last
commit of a branch instead of the last one.

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* Re: [PATCH 2/3] add new Git::Repo API
From: Jakub Narebski @ 2008-07-18 15:35 UTC (permalink / raw)
  To: Lea Wiemann; +Cc: git, John Hawley, Petr Baudis
In-Reply-To: <48809D31.5030008@gmail.com>

Lea Wiemann wrote:
> Jakub Narebski wrote:

>>>>> +=item $commit->message
>> 
>> I'd rather then have _git_ convert it to UTF=8 for us (using 
>> --encoding=<encoding> option to git-log/git-rev-list)
> 
> Yeah, I guess the API should actually decode it.  You wouldn't want to
> have the message in UTF-8 but in Unicode (I suggest you read man
> perlunitut if you haven't done so).

You mean perluniintro(1) here, isn't it?

Besides if decoding is done in Perl API, we can convert it simply
to Perl internal form (which, IIUC, in modern Perl is UTF-8 and
marked as such).

> We cannot have git do the decoding, 
> since (apart from the fact that it doesn't smell right) it isn't
> guaranteed to emit valid UTF-8 [...]

Well, if that is the case then Perl API has to do conversion, that
is the only sensible way.

>> It is (much) better than forking git-cat-file for each commit shown
>> on the list; nevertheless I think that it would be better to use git-log
>> to generate list (or Git::Revlist) of Git::Commit objects.  It is one
>> fork less, but what more important you don't have to access repository
>> twice for the very same objects.
> 
> You're confused; it's not one fork less, it's a write to a pipe less.
> (Pleeeease look at the code before you write something.  It's there, in
> this very thread.)  And I don't believe the "access the repository
> twice" thing is anywhere near an actual issue.  To summarize, you're
> asking me to (a) write code and (presumably) (b) add something to the
> interface of a public API, based on some (most probably faulty)
> assumptions about performance?  You should really read
> <http://c2.com/cgi/wiki?PrematureOptimization>.

Code is there, in gitweb, in parse_commits subroutine, or rather in
parse_commit_text subroutine.

[cut]

But I can agree that possible (and possibly minuscule) performance
improvement is not worth introducing new API and complicating (I think)
gitweb code.
 
>> I think that _not using_ Git::Cmd (or somesuch) API results in botched,
>> horrible API
>>   our $git_version = $repo_root->repo(directory => 'dummy')->version;
>> (Unless it is not needed any longer, or not used any longer; if it is
>> so, then perhaps implementing Git::Cmd as generic wrapper around git
>> commands, hiding for example ActivePerl hack, could be left for later).
> 
> It isn't used any longer -- I really suggest you read the whole thread
> before replying. ;-)

O.K.  Still I think that putting cmd_output and other in Git::Repo
is not a good API. I'd rather route calling git commands via Git or
Git::Cmd object (but Git::Repo would have Git/Git::Cmd object which
automatically adds '--git-dir=<path>', and possibly also
'--work-dir=<path>').

By the way, would you prefer if I commented on 3/3 patch as it is now,
taking into account what I remember from discussion on this and 2/3
patch (latter only as relevant), or would you rather I wait for next
round (next version) of patches?
 
>>> I wouldn't -- see my blurb about error handling at the top of my reply
>>> to Petr (<487BD0F3.2060508@gmail.com>).  You're not supposed to pass
>>> anything that you didn't get from get_sha1 into Git::Commit or
>>> Git::Tag constructors, or your error handling is invariably broken.
>> 
>> I can understand this simpler, although less than optimal, and geared
>> mainly towards gitweb needs.
> 
> FTR, yes it is simpler, but no, it is not really geared toward gitweb
> needs, and it's definitely not "less than optimal" in the sense of being
> worse than the exception-based error handling Git.pm does.  Trust on me
> on this one. ;-)

[...]

>> Why do you _need_ name_rev, if you are not to include git-describe
>> equivalent?
> 
> I needed it for gitweb.  As I said, I'm not trying to create a complete
> API.  A describe_rev (or so) method can be added later, if and when it's
> needed.  (As I said, I don't think writing APIs without at least one use
> case is a good idea anyway.)

Errr... I guess I misspoke. I should not say 'geared toward gitweb
needs', as perhaps it is 'created according [at least somewhat] to
what gitweb would need'.
 
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] Teach git submodule update to use distributed repositories
From: Nigel Magnay @ 2008-07-18 15:09 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <20080718144325.GR10151@machine.or.cz>

>> Ah - I understand. You're saying "you can't pull submodules when you
>> pull the supermodule, because you don't know which submodules might be
>> needed until you also merge / checkout the desired revision" ?
>>
>> Ack.
>
> That is something I might agree with, but my point is that within the
> submodule,
>
>        git pull
>
> simply isn't a sensible operation at all! You don't want to do any
> merges or whatever, just bring the submodule to a defined commit id.
> So you want to do something significantly different:
>
>        git fetch
>        git reset --hard <commitid>
>
> And that's what 'git submodule update' already does.
>

I wasn't wanting to do pull there - but either way, I agree :-)

>> Hm. It feels like each person could have some 'local' info in their
>> .gitmodules, and rules around merging; but I'm not sure of exactly
>> what, or how.
>
> Again, when customizing .gitmodules, you need to either give up on
>
>        (i) bisectability; it's not good enough to restore the canonical
>        .gitmodules contents on merge, since the bisect can run into one
>        of the commits on fred' branchs
>
>        (ii) publishing the exact same branch for testing and merging
>
> But I start to feel that the tradeoff of (ii) is really not so bad at
> alland this would be perhaps the most elegant solution. You can either
>
>        (a) make two parallel branches, one with your .gitmodules and
>        one with the upstream's
>
>        (b) probably better, stick a commit at the top of your branch
>        that will change .gitmodules to your locations; others can
>        check out fred, test things out, then merge fred^; you can even
>        generally go back in fred's commits if you just 'git submodule
>        update' right after checking fred out, since all the required
>        submodule commits will be probably already fetched.
>
> So I say just go for the (ii)-(b) combination. :-)
>

Hmm. Locally modifying my .gitmodules still feels bad because I don't
like either of those tradeoffs (but I don't have any sensible
suggestion yet).

As a bit of background (as to why I'd dislike (a) and (b)), we had a
team switch to git, and one of the really nice things is the ability
to share stuff around and branch freely - but the flipside of that is
that we tend to push to a central repo more rarely, so the advantages
of an continuous integration server become less. What we did is to
tell a centralised CI server the URLs of all the team's git
repositories, and it would periodically pull from them, speculatively
compile anything new, and run the big suite of tests - finishing up by
emailling them a heads-up that a particular state in their repo is
'bad'.

This was really popular as it was demonstrably better than anything we
could do with svn, and best of all, it's pretty much transparent - as
a user you don't have to do anything at all.

I could do it now by hacking about with files; it'd just be nice to
keep it transparent and make it a directly supported feature.

^ permalink raw reply

* Re: [PATCH] Teach git submodule update to use distributed repositories
From: Petr Baudis @ 2008-07-18 14:43 UTC (permalink / raw)
  To: Nigel Magnay; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <320075ff0807180420k4b28c317mc026713b22c44839@mail.gmail.com>

On Fri, Jul 18, 2008 at 12:20:13PM +0100, Nigel Magnay wrote:
> On Fri, Jul 18, 2008 at 11:00 AM, Petr Baudis <pasky@suse.cz> wrote:
> > On Fri, Jul 18, 2008 at 10:36:51AM +0100, Nigel Magnay wrote:
> >> On Fri, Jul 18, 2008 at 10:16 AM, Petr Baudis <pasky@suse.cz> wrote:
> >> > snip
> >> >
> >> >        "How do we mass-supply custom submodule URLs when publishing the
> >> >        customized main repository at a custom location too?"
> >> >
> >> Yes - that is an additional problem.
> >
> > Wait, I'm lost again - _additional_ problem? How does it differ from the
> > _original_ problem, how does it differ from what you're explaining below
> > and how does what you're explaining below differ from the original
> > problem?
> >
> In addition to the problem of needing to execute multiple commands and
> edit files to acheive what is a rather simple usecase, there is the
> additional problem of discovering (for a third party) a url for where
> their submodules are stored.

I see. That's interconnected as a single "How to check Fred's work"
problem for me. :-)

> >> If I may expand the usecase just so it's clear (and to check we're
> >> talkiing the same language)
> >>
> >> I do something like
> >> $ git remote add fred git://fredcomputer/superproject/.git
> >> $ git fetch --submodules fred
> >
> > I think you mean git pull --submodules fred. Well, actually, you want to
> > pull the main repository, then submodule update (_not_ pull in the
> > submodules). See? This is part of the "semantic swamp" I mentioned
> > before.
> 
> Ah - I understand. You're saying "you can't pull submodules when you
> pull the supermodule, because you don't know which submodules might be
> needed until you also merge / checkout the desired revision" ?
> 
> Ack.

That is something I might agree with, but my point is that within the
submodule,

	git pull

simply isn't a sensible operation at all! You don't want to do any
merges or whatever, just bring the submodule to a defined commit id.
So you want to do something significantly different:

	git fetch
	git reset --hard <commitid>

And that's what 'git submodule update' already does.

> Hm. It feels like each person could have some 'local' info in their
> .gitmodules, and rules around merging; but I'm not sure of exactly
> what, or how.

Again, when customizing .gitmodules, you need to either give up on

	(i) bisectability; it's not good enough to restore the canonical
	.gitmodules contents on merge, since the bisect can run into one
	of the commits on fred' branchs

	(ii) publishing the exact same branch for testing and merging

But I start to feel that the tradeoff of (ii) is really not so bad at
alland this would be perhaps the most elegant solution. You can either

	(a) make two parallel branches, one with your .gitmodules and
	one with the upstream's

	(b) probably better, stick a commit at the top of your branch
	that will change .gitmodules to your locations; others can
	check out fred, test things out, then merge fred^; you can even
	generally go back in fred's commits if you just 'git submodule
	update' right after checking fred out, since all the required
	submodule commits will be probably already fetched.

So I say just go for the (ii)-(b) combination. :-)

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* Re: Considering teaching plumbing to users harmful
From: J. Bruce Fields @ 2008-07-18 14:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vmykgfhtj.fsf@gitster.siamese.dyndns.org>

On Thu, Jul 17, 2008 at 12:10:00PM -0700, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Thu, 17 Jul 2008, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >> 
> >> >> Is there a way to commit the contents of a tarball without using 
> >> >> plumbing?  I occasionally want to track an upstream that I know only 
> >> >> as a series of tarballs, so I do something like:
> >> >> 
> >> >> 	cd repo/
> >> >> 	git checkout upstream
> >> >> 	rm -rf *
> >> >> 	tar -xzvf ../new-version.tar.gz
> >> >
> >> > How about "git add -u" and "git add ."?
> >> 
> >> It would work only if new version never removes files.
> >
> > You made me doubt for a second there.  But "git add -u" updates the index 
> > when a tracked files was deleted.  So after "rm -rf *", "git add -u" would 
> > empty the index.
> 
> I thought everybody would react to my message like so after sending it ;-)
> What I failed to say was that the main uneasiness about the above command
> sequence Bruce or anybody would have felt would be that "rm -fr *" step,
> which in itself look scary and does not remove .frotz that came from older
> version.

Yeah, good point, that's not very careful.

But actually it's "add -u" that I missed--I forgot it would take into
account removed files as well.  Thanks!

--b.

^ permalink raw reply

* [PATCH 1/2] archive: make zip compression level independent from core git
From: René Scharfe @ 2008-07-18 14:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

zlib_compression_level is the compression level used for git's object store.
It's 1 by default, which is the fastest setting.  This variable is also used
as the default compression level for ZIP archives created by git archive.

For archives, however, zlib's own default of 6 is more appropriate, as it's
favouring small size over speed -- archive creation is not that performance
critical most of the time.

This patch makes git archive independent from git's internal compression
level setting.  It affects invocations of git archive without explicitly
specified compression level option, only.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 archive-zip.c     |    9 +++++----
 archive.h         |    1 +
 builtin-archive.c |    3 ++-
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/archive-zip.c b/archive-zip.c
index d56e5cf..fb12398 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -93,7 +93,7 @@ static void copy_le32(unsigned char *dest, unsigned int n)
 }
 
 static void *zlib_deflate(void *data, unsigned long size,
-                          unsigned long *compressed_size)
+		int compression_level, unsigned long *compressed_size)
 {
 	z_stream stream;
 	unsigned long maxsize;
@@ -101,7 +101,7 @@ static void *zlib_deflate(void *data, unsigned long size,
 	int result;
 
 	memset(&stream, 0, sizeof(stream));
-	deflateInit(&stream, zlib_compression_level);
+	deflateInit(&stream, compression_level);
 	maxsize = deflateBound(&stream, size);
 	buffer = xmalloc(maxsize);
 
@@ -157,7 +157,7 @@ static int write_zip_entry(struct archiver_args *args,
 		method = 0;
 		attr2 = S_ISLNK(mode) ? ((mode | 0777) << 16) :
 			(mode & 0111) ? ((mode) << 16) : 0;
-		if (S_ISREG(mode) && zlib_compression_level != 0)
+		if (S_ISREG(mode) && args->compression_level != 0)
 			method = 8;
 		crc = crc32(crc, buffer, size);
 		out = buffer;
@@ -169,7 +169,8 @@ static int write_zip_entry(struct archiver_args *args,
 	}
 
 	if (method == 8) {
-		deflated = zlib_deflate(buffer, size, &compressed_size);
+		deflated = zlib_deflate(buffer, size, args->compression_level,
+				&compressed_size);
 		if (deflated && compressed_size - 6 < size) {
 			/* ZLIB --> raw compressed data (see RFC 1950) */
 			/* CMF and FLG ... */
diff --git a/archive.h b/archive.h
index 96bb1cd..4a02371 100644
--- a/archive.h
+++ b/archive.h
@@ -13,6 +13,7 @@ struct archiver_args {
 	time_t time;
 	const char **pathspec;
 	unsigned int verbose : 1;
+	int compression_level;
 };
 
 typedef int (*write_archive_fn_t)(struct archiver_args *);
diff --git a/builtin-archive.c b/builtin-archive.c
index d5e3af8..cff6ce7 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -185,9 +185,10 @@ int parse_archive_args(int argc, const char **argv, const struct archiver **ar,
 	if (!*ar)
 		die("Unknown archive format '%s'", format);
 
+	args->compression_level = Z_DEFAULT_COMPRESSION;
 	if (compression_level != -1) {
 		if ((*ar)->flags & USES_ZLIB_COMPRESSION)
-			zlib_compression_level = compression_level;
+			args->compression_level = compression_level;
 		else {
 			die("Argument not supported for format '%s': -%d",
 					format, compression_level);
-- 
1.5.6.2.212.g08b51

^ permalink raw reply related

* [PATCH 2/2] archive: remove unused headers
From: René Scharfe @ 2008-07-18 14:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

Remove obsolete #includes.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 archive-tar.c     |    2 --
 archive-zip.c     |    5 -----
 builtin-archive.c |    2 --
 3 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index f9eb726..1302961 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -2,9 +2,7 @@
  * Copyright (c) 2005, 2006 Rene Scharfe
  */
 #include "cache.h"
-#include "commit.h"
 #include "tar.h"
-#include "builtin.h"
 #include "archive.h"
 
 #define RECORDSIZE	(512)
diff --git a/archive-zip.c b/archive-zip.c
index fb12398..cf28504 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -2,11 +2,6 @@
  * Copyright (c) 2006 Rene Scharfe
  */
 #include "cache.h"
-#include "commit.h"
-#include "blob.h"
-#include "tree.h"
-#include "quote.h"
-#include "builtin.h"
 #include "archive.h"
 
 static int zip_date;
diff --git a/builtin-archive.c b/builtin-archive.c
index cff6ce7..df97724 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -7,10 +7,8 @@
 #include "archive.h"
 #include "commit.h"
 #include "tree-walk.h"
-#include "exec_cmd.h"
 #include "pkt-line.h"
 #include "sideband.h"
-#include "attr.h"
 
 static const char archive_usage[] = \
 "git archive --format=<fmt> [--prefix=<prefix>/] [--verbose] [<extra>] <tree-ish> [path...]";
-- 
1.5.6.2.212.g08b51

^ permalink raw reply related

* [PATCH] Documentation: How to ignore local changes in tracked files
From: Petr Baudis @ 2008-07-18 14:11 UTC (permalink / raw)
  To: gitster; +Cc: git
In-Reply-To: <20080717182619.GG10151@machine.or.cz>

This patch explains more carefully that `.gitignore` concerns only
untracked files and refers the reader to

	git update-index --assume-unchanged

in the need of ignoring uncommitted changes in already tracked files.
The description of this option is lifted to a more "porcelainish"
level and explains the caveats of this usecase.

Whether feasible or not, I believe adding this functionality to
the porcelain is out of the scope of this patch. (And I personally
think that referring to the plumbing in the case of such a special
usage is fine.)

This is currently probably one of the top FAQs at #git and the
--assume-unchanged switch is not widely known; gitignore(5) is the first
place where people are likely to look for it.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 Documentation/git-update-index.txt |   10 ++++++++++
 Documentation/gitignore.txt        |   11 ++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index a91fd21..6b930bc 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -88,6 +88,16 @@ OPTIONS
 	sometimes helpful when working with a big project on a
 	filesystem that has very slow lstat(2) system call
 	(e.g. cifs).
++
+This option can be also used as a coarse file-level mechanism
+to ignore uncommitted changes in tracked files (akin to what
+`.gitignore` does for untracked files).
+You should remember that an explicit 'git add' operation will
+still cause the file to be refreshed from the working tree.
+Git will fail (gracefully) in case it needs to modify this file
+in the index e.g. when merging in a commit;
+thus, in case the assumed-untracked file is changed upstream,
+you will need to handle the situation manually.
 
 -g::
 --again::
diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index fc0efd8..59321a2 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -13,9 +13,14 @@ DESCRIPTION
 -----------
 
 A `gitignore` file specifies intentionally untracked files that
-git should ignore.  Each line in a `gitignore` file specifies a
-pattern.
-
+git should ignore.
+Note that all the `gitignore` files really concern only files
+that are not already tracked by git;
+in order to ignore uncommitted changes in already tracked files,
+please refer to the 'git update-index --assume-unchanged'
+documentation.
+
+Each line in a `gitignore` file specifies a pattern.
 When deciding whether to ignore a path, git normally checks
 `gitignore` patterns from multiple sources, with the following
 order of precedence, from highest to lowest (within one level of

^ permalink raw reply related


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