Git development
 help / color / mirror / Atom feed
* Re: libgit2 - a true git library
From: Andreas Ericsson @ 2008-11-01 19:18 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Scott Chacon
In-Reply-To: <20081031170704.GU14786@spearce.org>

Shawn O. Pearce wrote:
> During the GitTogether we were kicking around the idea of a ground-up
> implementation of a Git library.  This may be easier than trying
> to grind down git.git into a library, as we aren't tied to any
> of the current global state baggage or the current die() based
> error handling.
> 
> I've started an _extremely_ rough draft.  The code compiles into a
> libgit.a but it doesn't even implement what it describes in the API,
> let alone a working Git implementation.  Really what I'm trying to
> incite here is some discussion on what the API looks like.
> 
> API Docs:
> http://www.spearce.org/projects/scm/libgit2/apidocs/html/modules.html
> 
> Source Code Clone URL:
> http://www.spearce.org/projects/scm/libgit2/libgit2.git
> 

Having looked briefly at the code, I've got a couple of comments:
* GIT_EXTERN() does nothing. Ever. It's noise and should be removed.
  Instead it would be better to have GIT_PRIVATE(), which could
  set visibility to "internal" or "hidden", meaning the symbol it's
  attached to can be used for lookups when creating a shared library
  but won't be usable from programs linking to that shared library
  (visibility-attributes have zero effect on static libraries). At
  least on all archs anyone really cares about.
* Prefixing the files themselves with git_ is useless and only leads
  to developer frustration. I imagine we'd be installing any header
  files in a git/ directory anyway, so we're gaining absolutely
  nothing with the git_ prefix on source-files.

Apart from that, it seems you've been designing a lot rather than
trying to use the API to actually do something. It would, imo, be
a lot better to start development with adding functionality shared
between all programs and then expand further on that, such as
incorporating all functions needed for manipulating tags into the
library and then modify existing code to use the library to get
tag-ish things done. That would also mean that the library would
quickly get used by core git, as once a certain part of it is
complete patches can be fitted to the library rather than to the
current non-libish dying() functions.


I also think it's quite alright to not strive *too* hard to make
all functions thread-safe, as very few of them will actually need
that. It's unlikely that a user program will spawn one thread to
write a lot of tags while another is trying to parse them, for
example.

By adding an init routine that determines the workdir and the
gitdir, one could start using the library straight away.

int git_init(const char *db, const char *worktree)
{
    if (git_set_db_dir(db))
        return -1;
    git_set_worktree((worktree))
        return -1;

    return 0;
}

and already you have a some few small helpers that are nifty to
to have around:
int git_is_gitdir(const char *path);  /* returns 1 on success */
int git_has_gitdir(const char *path); /* returns 1 on success */
const char *git_mkpath(const char *fmt, ...)

This way one will notice rather quickly what's needed (making it
easy to keep a more-or-less public TODO available, with small stuff
on it for the most part), and one can then go look for it in the
existing git code and, if possible, convert stuff or, best case
scenario, steal it straight off so that more apps can benefit from
tried and tested code.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: libgit2 - a true git library
From: Pierre Habouzit @ 2008-11-01 18:48 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Shawn O. Pearce, git, Scott Chacon
In-Reply-To: <490CA37C.1070107@op5.se>

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

On Sat, Nov 01, 2008 at 06:44:12PM +0000, Andreas Ericsson wrote:
> Pierre Habouzit wrote:

> >For types that _will_ be in the tight loops, we must make the types
> >explicit or it'll bite us hard performance-wise. I'm thinking what is
> >"struct object" or "struct commit" in git.git. It's likely that we will
> >loose a *lot* of those types are opaque.
> 
> The last sentence doesn't parse. I assume you mean "if those types 
> are..",

This was a typo, indeed s/of/if/

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* [PATCH v2] connect.c: add a way for git-daemon to pass an error back to client
From: Tom Preston-Werner @ 2008-11-01 18:44 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

The current behavior of git-daemon is to simply close the connection on
any error condition. This leaves the client without any information as
to the cause of the failed fetch/push/etc.

This patch allows get_remote_heads to accept a line prefixed with "ERR"
that it can display to the user in an informative fashion. Once clients
can understand this ERR line, git-daemon can be made to properly report
"repository not found", "permission denied", or other errors.

Example

S: ERR No matching repository.
C: fatal: remote error: No matching repository.

Signed-off-by: Tom Preston-Werner <tom@github.com>
---
Use prefixcmp instead of memcmp and test for "ERR " instead of "ERR"

 connect.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/connect.c b/connect.c
index 0c50d0a..584e04c 100644
--- a/connect.c
+++ b/connect.c
@@ -70,6 +70,9 @@ struct ref **get_remote_heads(int in, struct ref **list,
 		if (buffer[len-1] == '\n')
 			buffer[--len] = 0;

+		if (len > 4 && !prefixcmp(buffer, "ERR "))
+			die("remote error: %s", buffer + 4);
+
 		if (len < 42 || get_sha1_hex(buffer, old_sha1) || buffer[40] != ' ')
 			die("protocol error: expected sha/ref, got '%s'", buffer);
 		name = buffer + 41;
-- 
1.6.0.2

^ permalink raw reply related

* Re: libgit2 - a true git library
From: Andreas Ericsson @ 2008-11-01 18:44 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Shawn O. Pearce, git, Scott Chacon
In-Reply-To: <20081101173042.GE26229@artemis.corp>

Pierre Habouzit wrote:
> On Fri, Oct 31, 2008 at 06:41:54PM +0000, Shawn O. Pearce wrote:
>> How about this?
>>
>> http://www.spearce.org/projects/scm/libgit2/apidocs/CONVENTIONS
> 
> FWIW I've read what you say about types, while this is good design to
> make things abstract, accessors are slower _and_ disallow many
> optimizations as it's a function call and that it may clobber all your
> pointers values.
> 

Accessors are very nifty for one thing though; With a debugging flag,
you can use an accessor-function, while without that debugging flag you
can use a macro instead of a function. In other words, you use the
compiler as a sort of sanity-checker that you're only accessing the
variables through the proper macros.

This method introduces a bit of extra code (50% of which is always
dead) for each struct it's used on, but it makes debugging large-ish
pieces of software relatively simple, since access to all object types
is controlled through the use of macros.

> For types that _will_ be in the tight loops, we must make the types
> explicit or it'll bite us hard performance-wise. I'm thinking what is
> "struct object" or "struct commit" in git.git. It's likely that we will
> loose a *lot* of those types are opaque.
> 

The last sentence doesn't parse. I assume you mean "if those types are..",
in which case it'll be solved by using accessor-macros and forward-declaring
the structs.

> struct object in git has not changed since 2006.06. struct commit hasn't
> since 2005.04 if you ignore { unsigned int indegree; void *util; } that
> if I'm correct are annotations, and is a problem we (I think) have to
> address differently anyways (I gave my proposal on this, I'm eager to
> hear about what other think on the subject). So if in git.git that _is_
> a moving target we have had a 2 year old implementation for those types,
> it's that they're pretty well like this.
> 
> It's IMNSHO on the matter that core structures of git _will_ have to be
> made explicit. I'm thinking objects and their "subtypes" (commits,
> trees, blobs). Maybe a couple of things on the same vein.
> 

I agree. "git_commit", "git_tree", "git_blob" and "git_tag" can almost
certainly be set in stone straight away.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH] Documentation: add a planning document for the next CLI revamp
From: Elijah Newren @ 2008-11-01 18:36 UTC (permalink / raw)
  To: Sam Vilain; +Cc: git, Sam Vilain
In-Reply-To: <1225338485-11046-1-git-send-email-sam@vilain.net>

Hi,

Good list..  I agree with others that the 'undo' name doesn't sound
right (and will discuss other issues with it in response to another
email) but otherwise nice work.

On Wed, Oct 29, 2008 at 9:48 PM, Sam Vilain <sam@vilain.net> wrote:
> +  * 'git push' to checked out branch of non-bare repository not
> +    allowed without special configuration.  Configuration available
> +    that allows working directory to be updated, known caveats
> +    notwithstanding.  Ideally, it would refuse only in situations
> +    where a broken working copy would be left (because you couldn't
> +    fix it), and work when it can be known to be safe.

Configuration of remote repository, special command-line override, or both?


Some food for thought: One thing I did in EasyGit was to disallow
pushes to non-bare repositories* unless both source and destinations
references were explicitly specified.  For example:

$ eg push origin master    # or 'eg push', in this case
Aborting: You are trying to push to a repository with an associated working
copy, which will leave its working copy out of sync with its repository.
Rather than pushing changes to that repository, you should go to where that
repository is located and pull changes into it (using eg pull).  If you
know what you are doing and know how to deal with the consequences, you can
override this check by explicitly specifying source and destination
references, e.g.
  eg push REMOTE BRANCH:REMOTE_BRANCH
Please refer to
  eg help topic refspecs
to learn what this syntax means and what the consequences of overriding this
check are.

$ eg push origin master:master
Counting objects: 5, done.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (3/3), 260 bytes, done.
Total 3 (delta 1), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
To /home/newren/testing/dumb/.git
   852ffee..f5596e4  master -> master


This seems to prevent errors for new users, while still allowing
people to work around firewall issues.

* The big problem was that I was only able to detect if a remote
repository was bare or not if it was accessed via the local filesystem
or via ssh; for git:// (or rsync://) repositories I didn't know how to
perform such a check and so I simply omitted it.


Elijah

^ permalink raw reply

* Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client
From: Shawn O. Pearce @ 2008-11-01 18:10 UTC (permalink / raw)
  To: Tom Preston-Werner; +Cc: Junio C Hamano, git
In-Reply-To: <b97024a40810312329o53e37fd5td82aa69634ff1e6b@mail.gmail.com>

Tom Preston-Werner <tom@github.com> wrote:
> 
> I saw several methods of testing for a specific prefix in connect.c.
> Looking more closely at the source, the closest similar call is
> actually the test for ACK:
> 
> 	if (!prefixcmp(line, "ACK ")) {
> 		if (!get_sha1_hex(line+4, result_sha1)) {
> 			if (strstr(line+45, "continue"))
> 				return 2;
> 			return 1;
> 		}
> 	}
> 
> Explicitly testing for "ERR " (including the space) does seem like the
> more correct thing to do. Would you like me to resubmit a modified
> patch that uses prefixcmp()?

Yes, I think that is what Junio was hinting at.  The pattern above
is much more typical in Git sources, so keeping the new "ERR "
check consistent would be appreciated.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH v2] Introduce receive.denyDeletes
From: Shawn O. Pearce @ 2008-11-01 18:07 UTC (permalink / raw)
  To: Jan Krrrger; +Cc: Junio C Hamano, git
In-Reply-To: <20081101154216.45021eee@perceptron>

Jan Krrrger <jk@jk.gs> wrote:
> Occasionally, it may be useful to prevent branches from getting deleted from
> a centralized repository, particularly when no administrative access to the
> server is available to undo it via reflog. It also makes
> receive.denyNonFastForwards more useful if it is used for access control
> since it prevents force-updating by deleting and re-creating a ref.
> 
> Signed-off-by: Jan Krüger <jk@jk.gs>
> ---
> Like this, then?

Acked-by: Shawn O. Pearce <spearce@spearce.org>
 
>  Documentation/config.txt |    4 ++++
>  builtin-receive-pack.c   |   12 ++++++++++++
>  t/t5400-send-pack.sh     |   11 +++++++++++
>  3 files changed, 27 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 29369d0..965ed74 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1188,6 +1188,10 @@ receive.unpackLimit::
>  	especially on slow filesystems.  If not set, the value of
>  	`transfer.unpackLimit` is used instead.
>  
> +receive.denyDeletes::
> +	If set to true, git-receive-pack will deny a ref update that deletes
> +	the ref. Use this to prevent such a ref deletion via a push.
> +
>  receive.denyNonFastForwards::
>  	If set to true, git-receive-pack will deny a ref update which is
>  	not a fast forward. Use this to prevent such an update via a push,
> diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
> index 9f60f31..2c0225c 100644
> --- a/builtin-receive-pack.c
> +++ b/builtin-receive-pack.c
> @@ -11,6 +11,7 @@
>  
>  static const char receive_pack_usage[] = "git-receive-pack <git-dir>";
>  
> +static int deny_deletes = 0;
>  static int deny_non_fast_forwards = 0;
>  static int receive_fsck_objects;
>  static int receive_unpack_limit = -1;
> @@ -23,6 +24,11 @@ static int capabilities_sent;
>  
>  static int receive_pack_config(const char *var, const char *value, void *cb)
>  {
> +	if (strcmp(var, "receive.denydeletes") == 0) {
> +		deny_deletes = git_config_bool(var, value);
> +		return 0;
> +	}
> +
>  	if (strcmp(var, "receive.denynonfastforwards") == 0) {
>  		deny_non_fast_forwards = git_config_bool(var, value);
>  		return 0;
> @@ -185,6 +191,12 @@ static const char *update(struct command *cmd)
>  		      "but I can't find it!", sha1_to_hex(new_sha1));
>  		return "bad pack";
>  	}
> +	if (deny_deletes && is_null_sha1(new_sha1) &&
> +	    !is_null_sha1(old_sha1) &&
> +	    !prefixcmp(name, "refs/heads/")) {
> +		error("denying ref deletion for %s", name);
> +		return "deletion prohibited";
> +	}
>  	if (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&
>  	    !is_null_sha1(old_sha1) &&
>  	    !prefixcmp(name, "refs/heads/")) {
> diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
> index 544771d..6fe2f87 100755
> --- a/t/t5400-send-pack.sh
> +++ b/t/t5400-send-pack.sh
> @@ -103,6 +103,17 @@ unset GIT_CONFIG GIT_CONFIG_LOCAL
>  HOME=`pwd`/no-such-directory
>  export HOME ;# this way we force the victim/.git/config to be used.
>  
> +test_expect_failure \
> +	'pushing a delete should be denied with denyDeletes' '
> +	cd victim &&
> +	git config receive.denyDeletes true &&
> +	git branch extra master &&
> +	cd .. &&
> +	test -f victim/.git/refs/heads/extra &&
> +	test_must_fail git send-pack ./victim/.git/ :extra master
> +'
> +rm -f victim/.git/refs/heads/extra
> +
>  test_expect_success \
>          'pushing with --force should be denied with denyNonFastforwards' '
>  	cd victim &&
> -- 
> 1.6.0.3.524.g86cf.dirty

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 2/3] git send-email: do not ask questions when  --compose is used.
From: Ian Hilt @ 2008-11-01 17:54 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git
In-Reply-To: <20081101170817.GC26229@artemis.corp>

On Sat, 1 Nov 2008, Pierre Habouzit wrote:
> On Sat, Nov 01, 2008 at 01:00:33PM +0000, Ian Hilt wrote:
> > I'm confused here.  Why would a MIME library help?
> 
> Hmm maybe I'm wrong, but the idea would be to do what mutt does and be
> able to parse:
> 
> To: John Doe <some.address@some.tld>, Random Joe <random.joe@abc.def>,
>   Superman <batman@nyc.us>,
>   "Someone with a comma, inside its tag name" <a@b.com>

No, you're absolutely right for this reason: MIME-encoded addresses.

Ugh.

Although, to my knowledge, git-send-email doesn't understand that now. 
So unless we want to support MIME-encoded addresses, this won't be
necessary.

> And that needs to know how to do that with perl, and _really_ I hate
> perl enough for not being able to do that well. Splitting on ',' is just
> not going to fly.

Yea, it's going to be a bit trickier than that.

> > > But yeah, I knew I left out those, and this was the reason.
> > 
> > Anyway, do you, or does anyone else, think it's even worth coding the
> > possibility for the user to edit the To and Cc fields?
> 
> *YES*
> 
> I would love to see git-send-email work like mutt does: it fills the
> field like it does now, and you are allowed to fix that, and it parses
> the buffer back to guess what you wanted. It allow to drop most of the
> interactive prompting that is so annoying (since it's not in-shell and
> has no history and stuff like that, unlike my $EDITOR).

Hmm, I'll look into this; especially the MIME library.  I'm not a perl
monk, but I can give it a shot, no?


	Ian

^ permalink raw reply

* Re: Git/Mercurial interoperability (and what about bzr?)
From: Linus Torvalds @ 2008-11-01 17:51 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Florian Weimer, Jakub Narebski, git
In-Reply-To: <20081101133931.GC8134@mit.edu>



On Sat, 1 Nov 2008, Theodore Tso wrote:
> 
> .hgtags is stored as a versioned file in Mercurial.  That's one of the
> problems, and it leads to no shortage of headaches.

I told people this was insane long long ago, and I thought the hg people 
had learnt to use local tags. They act sanely, as far as I know (ie they 
act the same way git tags do).

Of course, the problem with hg local tags is that hg apparently has no 
sane way to _propagate_ such local tag-space information from one 
repository to another. But that's purely a problem with hg itself. I don't 
know why that hasn't gotten fixed.

			Linus

^ permalink raw reply

* Re: [PATCH 2/3] git send-email: do not ask questions when  --compose is used.
From: Pierre Habouzit @ 2008-11-01 17:43 UTC (permalink / raw)
  To: Francis Galiegue; +Cc: git
In-Reply-To: <200811011834.32702.fg@one2team.com>

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

On Sat, Nov 01, 2008 at 05:34:32PM +0000, Francis Galiegue wrote:
> Le Saturday 01 November 2008 18:08:17 Pierre Habouzit, vous avez écrit :
> [...]
> >
> > To: John Doe <some.address@some.tld>, Random Joe <random.joe@abc.def>,
> >   Superman <batman@nyc.us>,
> >   "Someone with a comma, inside its tag name" <a@b.com>
> >
> 
> fg@erwin ~ $ cat t.txt
> To: John Doe <some.address@some.tld>, Random Joe <random.joe@abc.def>, 
> Superman <batman@nyc.us>, "Someone with a comma, inside its tag name" 
> <a@b.com>
> fg@erwin ~ $ <t.txt perl -ne 's,^To:\s*,,i; @mails = m/\s*((?:"[^"]+")?
> \s*<[^@]+@[^@]+>)\s*,?/g; END { print join("\n", @mails) . "\n"}'
> <some.address@some.tld>
> <random.joe@abc.def>
> <batman@nyc.us>
> "Someone with a comma, inside its tag name" <a@b.com>
> 
> That's regex, not especially perl ;)

Your regex fails to parse:

"Someone with a comma, and an escape double quote \" in its name"
  <regex.cant.be.used.for.serious.parsing@i.told.you.so>

That's why I first hinted at some MIME library that has probably made
that right.

Not to mention that you don't fix the multiline issue, but that is quite
less of a problem, it merely needs an accumulator and a 1 line lookahead
in the parser code while in the header parts. Nothing unrealistic for me
to write.

But I just can't resolve myself to parse anything with regex because I
just know it's horribly broken and wrong.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* Re: [PATCH 2/3] git send-email: do not ask questions when  --compose is used.
From: Francis Galiegue @ 2008-11-01 17:34 UTC (permalink / raw)
  To: git
In-Reply-To: <20081101170817.GC26229@artemis.corp>

Le Saturday 01 November 2008 18:08:17 Pierre Habouzit, vous avez écrit :
[...]
>
> To: John Doe <some.address@some.tld>, Random Joe <random.joe@abc.def>,
>   Superman <batman@nyc.us>,
>   "Someone with a comma, inside its tag name" <a@b.com>
>

fg@erwin ~ $ cat t.txt
To: John Doe <some.address@some.tld>, Random Joe <random.joe@abc.def>, 
Superman <batman@nyc.us>, "Someone with a comma, inside its tag name" 
<a@b.com>
fg@erwin ~ $ <t.txt perl -ne 's,^To:\s*,,i; @mails = m/\s*((?:"[^"]+")?
\s*<[^@]+@[^@]+>)\s*,?/g; END { print join("\n", @mails) . "\n"}'
<some.address@some.tld>
<random.joe@abc.def>
<batman@nyc.us>
"Someone with a comma, inside its tag name" <a@b.com>

That's regex, not especially perl ;)

-- 
Francis Galiegue
ONE2TEAM
Ingénieur système
Mob : +33 (0) 6 83 87 78 75
Tel : +33 (0) 1 78 94 55 52
fge@one2team.com
40 avenue Raymond Poincaré
75116 Paris

^ permalink raw reply

* Re: libgit2 - a true git library
From: Pierre Habouzit @ 2008-11-01 17:30 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Scott Chacon
In-Reply-To: <20081031184154.GV14786@spearce.org>

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

On Fri, Oct 31, 2008 at 06:41:54PM +0000, Shawn O. Pearce wrote:
> How about this?
> 
> http://www.spearce.org/projects/scm/libgit2/apidocs/CONVENTIONS

FWIW I've read what you say about types, while this is good design to
make things abstract, accessors are slower _and_ disallow many
optimizations as it's a function call and that it may clobber all your
pointers values.

For types that _will_ be in the tight loops, we must make the types
explicit or it'll bite us hard performance-wise. I'm thinking what is
"struct object" or "struct commit" in git.git. It's likely that we will
loose a *lot* of those types are opaque.

struct object in git has not changed since 2006.06. struct commit hasn't
since 2005.04 if you ignore { unsigned int indegree; void *util; } that
if I'm correct are annotations, and is a problem we (I think) have to
address differently anyways (I gave my proposal on this, I'm eager to
hear about what other think on the subject). So if in git.git that _is_
a moving target we have had a 2 year old implementation for those types,
it's that they're pretty well like this.

It's IMNSHO on the matter that core structures of git _will_ have to be
made explicit. I'm thinking objects and their "subtypes" (commits,
trees, blobs). Maybe a couple of things on the same vein.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
From: Tuncer Ayaz @ 2008-11-01 17:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <7vprllh0ao.fsf@gitster.siamese.dyndns.org>

On Tue, Oct 28, 2008 at 4:21 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nanako Shiraishi <nanako3@lavabit.com> writes:
>
>> Quoting Tuncer Ayaz <tuncer.ayaz@gmail.com>:
>>
>>> After fixing clone -q I noticed that pull -q does not do what
>>> it's supposed to do and implemented --quiet/--verbose by
>>> adding it to builtin-merge and fixing two places in builtin-fetch.
>>
>> Junio, may I ask what the status of this patch is? Maybe the patch was
>> lost in the noise? The commit log message is written very differently from
>> existing commits in the history of git, and I am thinking that maybe that is
>> why you did not like the whole patch? Or is it lack of any test script?
>
> Well, perhaps you've been with us long enough to get too picky like
> myself, but this was genuinely lost in the noise and my scrambling to get
> back to normal.  We do not typically say "I did this, I did that", but the
> first paragraph in Tuncer's message is perfectly fine.  I would probably
> liked the second paragraph better if it were after --- lines (it is more
> about the possible enhancements in other areas, and does not belong to the
> commit log message for this change), but it is not a grave enough offence
> to get the patch rejected.

Should I resend the patch with a short and simple commit message
like the following?
--8<--
Implement git-pull --quiet and --verbose by adding the
options to git-pull and fixing verbosity handling in git-fetch.
-->8--

> The patch itself looks Ok; the lack of test script additions does indeed
> bother me somewhat, but it is not too big a deal.

I took a look at t/ and am not quite sure whether I should test -v/-q
by analyzing stdout output's length & content.

I think testing -q and -v makes more sense on a global and not
git-pull or git-fetch level. For that to be possible I envision building
common verbosity-based logging functions/macros.

I don't like the idea of scanning stdout for length or content as
long as we're not sure that all errors and warnings are sent
to stderr and stdout is for info and verbose only.

> P.S. We are having fun at GitTogether'08 in the first half of this week,
> so please expect slower response than usual.

^ permalink raw reply

* Re: [PATCH 2/3] git send-email: do not ask questions when  --compose is used.
From: Pierre Habouzit @ 2008-11-01 17:08 UTC (permalink / raw)
  To: Ian Hilt; +Cc: git
In-Reply-To: <20081101130033.GD17961@sys-0.hiltweb.site>

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

On Sat, Nov 01, 2008 at 01:00:33PM +0000, Ian Hilt wrote:
> On Sat, Nov 01, 2008 at 12:04:39PM +0100, Pierre Habouzit wrote:
> > I didn't do it for a very good reason: the To field is tricker to parse
> > because very fast it's multiline, and must be split along the ',' when
> > parsed back and so on.
> 
> Right.  So my patch is broken in that it doesn't parse the addresses
> correctly.  This _should_ be easy to fix.  I knew my patch sucked, but I
> wanted to get the idea out there.  For me, I don't like specifying all
> that information on the command-line.  It would be nice to be able to
> edit the To and Cc fields in the editor.
> 
> > And even moreuseful than the To is the Cc list that git-send-email
> > bloats to death and that I would like to reduce very often.
> 
> You mean git-send-email adds too many addresses to the Cc list, or the
> code for those fields is already bloated to death?

The former, I've not looked at the code I can't really say.

> > But sadly that needs an expertise of perl I absolutely don't have. We
> > probably even want to depend on some MIME perl library that knows about
> > those kind of issues and do it for us well.
> 
> I'm confused here.  Why would a MIME library help?

Hmm maybe I'm wrong, but the idea would be to do what mutt does and be
able to parse:

To: John Doe <some.address@some.tld>, Random Joe <random.joe@abc.def>,
  Superman <batman@nyc.us>,
  "Someone with a comma, inside its tag name" <a@b.com>

And that needs to know how to do that with perl, and _really_ I hate
perl enough for not being able to do that well. Splitting on ',' is just
not going to fly.

> > But yeah, I knew I left out those, and this was the reason.
> 
> Anyway, do you, or does anyone else, think it's even worth coding the
> possibility for the user to edit the To and Cc fields?

*YES*

I would love to see git-send-email work like mutt does: it fills the
field like it does now, and you are allowed to fix that, and it parses
the buffer back to guess what you wanted. It allow to drop most of the
interactive prompting that is so annoying (since it's not in-shell and
has no history and stuff like that, unlike my $EDITOR).

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* Re: libgit2 - a true git library
From: Pierre Habouzit @ 2008-11-01 17:01 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Johannes Schindelin, Shawn O. Pearce, Junio C Hamano,
	Pieter de Bie, git, Scott Chacon
In-Reply-To: <alpine.LFD.2.00.0811010944000.13034@xanadu.home>

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

On Sat, Nov 01, 2008 at 01:50:45PM +0000, Nicolas Pitre wrote:
> On Sat, 1 Nov 2008, Pierre Habouzit wrote:
> 
> > Well, you can't return _sanely_ an error through a pointer. The &1
> > method is broken as soon as you return a char* (there is an alignment
> > requirement for malloc, not for any pointer out there), hence shall not
> > be used, as it would not be the sole way to test for error.
> > 
> > Another option, that is _theorically_ not portable, but is ttbomk on all
> > the platforms we intend to support (IOW POSIX-ish and windows), is to
> > use "small" values of the pointers for errors. [NULL .. (void *)(PAGE_SIZE - 1)[
> > cannot exist, which gives us probably always 512 different errors, and
> 
> 4095 actually.  You don't need to align error codes.

Sure, I'm just not sure there isn't an arch where a page would be 512
octets. And you really have 4096 errors, from 0 to 4095 *included* :)

> > the test is ((uintptr_t)ptr < (PAGE_SIZE)) which is cheap. It's butt
> > ugly, but encoding errors into pointers is butt ugly in the first place.
> 
> Or use "negative" pointers.  Again, please have a look at 
> include/linux/err.h.  The pointer range from 0xffffffff (or 
> 0xffffffffffffffff on 64-bit machines) down to the range you want is for 
> errors, and the top of the address range is almost certain to never be 
> valid in user space either.

Indeed.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* [PATCH v2] Introduce receive.denyDeletes
From: Jan Krüger @ 2008-11-01 14:42 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20081031143022.GQ14786@spearce.org>

Occasionally, it may be useful to prevent branches from getting deleted from
a centralized repository, particularly when no administrative access to the
server is available to undo it via reflog. It also makes
receive.denyNonFastForwards more useful if it is used for access control
since it prevents force-updating by deleting and re-creating a ref.

Signed-off-by: Jan Krüger <jk@jk.gs>
---
Like this, then?

 Documentation/config.txt |    4 ++++
 builtin-receive-pack.c   |   12 ++++++++++++
 t/t5400-send-pack.sh     |   11 +++++++++++
 3 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 29369d0..965ed74 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1188,6 +1188,10 @@ receive.unpackLimit::
 	especially on slow filesystems.  If not set, the value of
 	`transfer.unpackLimit` is used instead.
 
+receive.denyDeletes::
+	If set to true, git-receive-pack will deny a ref update that deletes
+	the ref. Use this to prevent such a ref deletion via a push.
+
 receive.denyNonFastForwards::
 	If set to true, git-receive-pack will deny a ref update which is
 	not a fast forward. Use this to prevent such an update via a push,
diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index 9f60f31..2c0225c 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -11,6 +11,7 @@
 
 static const char receive_pack_usage[] = "git-receive-pack <git-dir>";
 
+static int deny_deletes = 0;
 static int deny_non_fast_forwards = 0;
 static int receive_fsck_objects;
 static int receive_unpack_limit = -1;
@@ -23,6 +24,11 @@ static int capabilities_sent;
 
 static int receive_pack_config(const char *var, const char *value, void *cb)
 {
+	if (strcmp(var, "receive.denydeletes") == 0) {
+		deny_deletes = git_config_bool(var, value);
+		return 0;
+	}
+
 	if (strcmp(var, "receive.denynonfastforwards") == 0) {
 		deny_non_fast_forwards = git_config_bool(var, value);
 		return 0;
@@ -185,6 +191,12 @@ static const char *update(struct command *cmd)
 		      "but I can't find it!", sha1_to_hex(new_sha1));
 		return "bad pack";
 	}
+	if (deny_deletes && is_null_sha1(new_sha1) &&
+	    !is_null_sha1(old_sha1) &&
+	    !prefixcmp(name, "refs/heads/")) {
+		error("denying ref deletion for %s", name);
+		return "deletion prohibited";
+	}
 	if (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&
 	    !is_null_sha1(old_sha1) &&
 	    !prefixcmp(name, "refs/heads/")) {
diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index 544771d..6fe2f87 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -103,6 +103,17 @@ unset GIT_CONFIG GIT_CONFIG_LOCAL
 HOME=`pwd`/no-such-directory
 export HOME ;# this way we force the victim/.git/config to be used.
 
+test_expect_failure \
+	'pushing a delete should be denied with denyDeletes' '
+	cd victim &&
+	git config receive.denyDeletes true &&
+	git branch extra master &&
+	cd .. &&
+	test -f victim/.git/refs/heads/extra &&
+	test_must_fail git send-pack ./victim/.git/ :extra master
+'
+rm -f victim/.git/refs/heads/extra
+
 test_expect_success \
         'pushing with --force should be denied with denyNonFastforwards' '
 	cd victim &&
-- 
1.6.0.3.524.g86cf.dirty

^ permalink raw reply related

* Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client
From: Alex Riesen @ 2008-11-01 14:39 UTC (permalink / raw)
  To: Tom Preston-Werner; +Cc: Johannes Schindelin, Nicolas Pitre, git, gitster
In-Reply-To: <b97024a40810312035v5416b578v51b5bed528ca8d39@mail.gmail.com>

Tom Preston-Werner, Sat, Nov 01, 2008 04:35:20 +0100:
> Maybe this is worth asking the question: does anybody use git-daemon
> for private code? If so, why are they not using SSH instead? And in
> that case, how are informative error messages a security risk?

Yes. I use both in my private network, with only ssh open to the
internet. git-daemon is smaller and faster (started from inetd). And
I'm absolutely sure wont ever accidentally push something in the
mirrored repos.

I never had the error reporting problem in this setup, though. It is a
fully controled environment and I can just look in syslog.

I support the original reason for not doing the errors, BTW. It cannot
be on by default.

Heh, try the patch for your private repos and private repos of your
employer, who can sack you for exposing confidential information, and
open them to internet. Than come back and tell us how safe you feel :)

^ permalink raw reply

* Re: [PATCH] Use find instead of perl in t5000 to get file modification time
From: Alex Riesen @ 2008-11-01 14:24 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Sam Vilain, Git Mailing List, Junio C Hamano, Jeff King,
	René Scharfe
In-Reply-To: <alpine.DEB.1.00.0811010121570.22125@pacific.mpi-cbg.de.mpi-cbg.de>

Johannes Schindelin, Sat, Nov 01, 2008 01:23:32 +0100:
> 
> Well, if you install Git for Windows (as opposed to cygwin), it is minimum 
> hassle, and Perl is delivered right with it.
> 

I'd like to try it again, but weren't ther some fatal problems with
cygwin1.dll being in PATH? I always work either in Cygwin's bash or
just have to have it in PATH, because of the build environment even
being strictly Windows based (case-insensitive and alike) just have to
use sane tooling in its scripts.

> 
> P.S.: some guys at the GSoC mentor summit convinced me in at least trying 
> to fix _their_ problems on msysGit, so chances are good I'll fix issues 
> you would encounter in the same run.
> 

Do you still plan to distribute MinGW with it? It's very nice to be
able to track Junio's repo, have own branches and rebuild Git from
time to time. For me, at least.

^ permalink raw reply

* Re: libgit2 - a true git library
From: Nicolas Pitre @ 2008-11-01 13:50 UTC (permalink / raw)
  To: Pierre Habouzit
  Cc: Johannes Schindelin, Shawn O. Pearce, Junio C Hamano,
	Pieter de Bie, git, Scott Chacon
In-Reply-To: <20081101110120.GA3819@artemis.corp>

On Sat, 1 Nov 2008, Pierre Habouzit wrote:

> Well, you can't return _sanely_ an error through a pointer. The &1
> method is broken as soon as you return a char* (there is an alignment
> requirement for malloc, not for any pointer out there), hence shall not
> be used, as it would not be the sole way to test for error.
> 
> Another option, that is _theorically_ not portable, but is ttbomk on all
> the platforms we intend to support (IOW POSIX-ish and windows), is to
> use "small" values of the pointers for errors. [NULL .. (void *)(PAGE_SIZE - 1)[
> cannot exist, which gives us probably always 512 different errors, and

4095 actually.  You don't need to align error codes.

> the test is ((uintptr_t)ptr < (PAGE_SIZE)) which is cheap. It's butt
> ugly, but encoding errors into pointers is butt ugly in the first place.

Or use "negative" pointers.  Again, please have a look at 
include/linux/err.h.  The pointer range from 0xffffffff (or 
0xffffffffffffffff on 64-bit machines) down to the range you want is for 
errors, and the top of the address range is almost certain to never be 
valid in user space either.


Nicolas

^ permalink raw reply

* Re: Git/Mercurial interoperability (and what about bzr?)
From: Theodore Tso @ 2008-11-01 13:39 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Jakub Narebski, git
In-Reply-To: <87abcjpvy2.fsf@mid.deneb.enyo.de>

On Sat, Nov 01, 2008 at 11:44:21AM +0100, Florian Weimer wrote:
> > Now Mercurial has chosen to use in-tree '.hgtags' file to have global
> > tags transferable.  Never mind the fact that it had to treat this file
> > in special way to have it non-versioned
> 
> Oops, thought this file was versioned.  Things like
> 
>   <http://tycoon.hpl.hp.com/changeset/932:931d181e9f58/.hgtags>

.hgtags is stored as a versioned file in Mercurial.  That's one of the
problems, and it leads to no shortage of headaches.

Some of the problems are discussed here:

   http://www.selenic.com/mercurial/wiki/index.cgi/Tag

Specifically, to quote the Mercurial Wiki:

    The fact that tags identify changesets and are also parts of
    changesets has some potentially confusing implications:

    * The changeset that a tag refers to is always older than the
      changeset that commits the tag itself.

    * Updating a working dir to a particular tag will take that
      directory back to a point before the tag itself existed.

    * Cloning a repo to a particular tag will give you a new repo that
      does not have that tag.

In addition, Mercurial has to play interesting special case games in a
repository which has multiple heads (in git terms, "branches").  When
looking up a tag, it has to take the union of all of the .hgtags files
at the tips of each of the branches.  So if you have a large number of
heads in your Mercurial repository, tags access doesn't scale well at
all.  Mercurial is very much optimized for having a single or at best
a few heads/branches in a repository.

As I mentioned earlier this makes it much more difficult to do a
bidrectional hg/git gateway, since the two DAG's are not toplogically
equivalent, and in fact, *when* you add a tag to a particular
commit/revision can make a distinct difference to the shape of the
DAG.  If you tag right after creating the revision in question, then
there is a tag commit right after the revision.  If you commit a few
dozen changes into the repository, and *then* tag a point many commits
earlier, then the tag commit will be tacked onto the head of whatever
branch you happened to be on.

In fact, in the worst case, if you accidentally tag a revision on the
"maint" branch while you happened to be on the "devel" branch, the tag
for the commit in the "maint" branch will be attached to the "devel"
branch, and while it will work just fine for *you*, someone who only
fetches the "maint" branch might never see the tag that you placed on
the maint branch --- unless they happen to also pull the devel branch.

What fun, eh?

					- Ted

^ permalink raw reply

* Re: Are binary xdeltas only used if you use git-gc?
From: Nicolas Pitre @ 2008-11-01 13:25 UTC (permalink / raw)
  To: Thanassis Tsiodras; +Cc: Matthieu Moy, Jakub Narebski, git
In-Reply-To: <f1d2d9ca0811010454u203a7c88x1e09735b3fc1358f@mail.gmail.com>

On Sat, 1 Nov 2008, Thanassis Tsiodras wrote:

> Thanks to everybody for your help.
> 
> I will setup an alias to always use "git push --thin".
> For the reverse direction, I don't see a --thin for "git pull",
> 
> My understanding is that "git pull" is optimal,
> and does what --thin does for push anyway, right?

Exact.


Nicolas

^ permalink raw reply

* Re: [PATCH 2/3] git send-email: do not ask questions when --compose is used.
From: Ian Hilt @ 2008-11-01 13:00 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git
In-Reply-To: <20081101110439.GB3819@artemis.corp>

On Sat, Nov 01, 2008 at 12:04:39PM +0100, Pierre Habouzit wrote:
> I didn't do it for a very good reason: the To field is tricker to parse
> because very fast it's multiline, and must be split along the ',' when
> parsed back and so on.

Right.  So my patch is broken in that it doesn't parse the addresses
correctly.  This _should_ be easy to fix.  I knew my patch sucked, but I
wanted to get the idea out there.  For me, I don't like specifying all
that information on the command-line.  It would be nice to be able to
edit the To and Cc fields in the editor.

> And even moreuseful than the To is the Cc list that git-send-email
> bloats to death and that I would like to reduce very often.

You mean git-send-email adds too many addresses to the Cc list, or the
code for those fields is already bloated to death?

> But sadly that needs an expertise of perl I absolutely don't have. We
> probably even want to depend on some MIME perl library that knows about
> those kind of issues and do it for us well.

I'm confused here.  Why would a MIME library help?

> But yeah, I knew I left out those, and this was the reason.

Anyway, do you, or does anyone else, think it's even worth coding the
possibility for the user to edit the To and Cc fields?


	Ian

^ permalink raw reply

* Re: [PATHv2 7/8] gitweb: embed snapshot format parameter in PATH_INFO
From: Giuseppe Bilotta @ 2008-11-01 12:57 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <200811010118.14191.jnareb@gmail.com>

On Sat, Nov 1, 2008 at 1:18 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Sun, 19 Oct 2008, Giuseppe Bilotta wrote:
>
> I'm sorry for the delay.

No problem.

>> When PATH_INFO is active, get rid of the sf CGI parameter by embedding
>> the snapshot format information in the PATH_INFO URL, in the form of an
>> appropriate extension.
>
> The question is: should we use format suffix (e.g. 'tar.gz'),
> or format name (e.g. 'tgz')?  The latter is later easier to parse,
> see comments to first patch in better snapshot support for path_info.

I think it's essentially a matter of deciding which extension to use
by default on output, and accepting either extension on input.


>> +             # since we destructively absorb parameters, we keep this
>> +             # boolean that remembers if we're handling a snapshot
>> +             my $is_snapshot = $params{'action'} eq 'snapshot';
>> +
>
> Side note: we destructively absorb parameters, because parameters
> which are not absorbed are then used to generate query string part
> of URL.
>
> Deleting parameter but remembering the fact that it was used is one
> (but not only) solution.

Well, in the specific case of the 'action' parameter we could just
keep it around and only discard it at the end.

>> +                     if (!$fmt) {
>> +                             my @snapshot_fmts = gitweb_check_feature('snapshot');
>> +                             @snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);
>> +                             $fmt = $snapshot_fmts[0];
>> +                     }
>
> I anderstand that the above code is improved with new patch?

Yup, I followed your suggestion of putting the refactoring earlier.

>> +                     $href .= $known_snapshot_formats{$fmt}{'suffix'};
>
> Again: should we use snapshot prefix, or snapshot name, which means here
> do we use $known_snapshot_formats{$fmt}{'suffix'}; or just $fmt; ?

I really don't know, either is fine for me.

Maybe using $fmt has the benefit that it copes better with some buggy
versions of some browser (like Opera) that tend to strip the .gz from
.tar.gz files ...


-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply

* Re: Are binary xdeltas only used if you use git-gc?
From: Thanassis Tsiodras @ 2008-11-01 11:54 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Matthieu Moy, Jakub Narebski, git
In-Reply-To: <alpine.LFD.2.00.0810311549570.13034@xanadu.home>

Thanks to everybody for your help.

I will setup an alias to always use "git push --thin".
For the reverse direction, I don't see a --thin for "git pull",

My understanding is that "git pull" is optimal,
and does what --thin does for push anyway, right?

On 10/31/08, Nicolas Pitre <nico@cam.org> wrote:
> On Fri, 31 Oct 2008, Matthieu Moy wrote:
>
>> Jakub Narebski <jnareb@gmail.com> writes:
>>
>> > Thanassis Tsiodras wrote:
>> >
>> >> So I have to git-gc on my side (after the commits), git-gc on the
>> >> remote,
>> >> and then git-push?
>> >
>> > Perhaps I haven't made myself clear.
>> >
>> > On the local side: git-commit creates loose (compressed, but not
>> > deltified) objects. git-gc packs and deltifies.
>> >
>> > On the remote side (for smart protocols, i.e. git and ssh): git
>> > creates _thin_ pack, deltified;
>>
>> I don't understand this point: the OP talks about pushing, so isn't
>> the pack created on the _local_ machine (and then sent to the remote)?
>
> Yes, the pack is created on the fly when pushing, regardless if the repo
> is already packed or not locally.  The only difference a locally packed
> repo provides is a shorter "Compressing objects" phase when pushing
> that's all. The packedness of the remote has no effect at all.
>
>
> Nicolas
>


-- 
What I gave, I have; what I spent, I had; what I kept, I lost. -Old Epitaph

^ permalink raw reply

* Re: [PATCH] prepare deprecation of git-revert
From: Andreas Ericsson @ 2008-11-01 11:53 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Jakub Narebski, Pierre Habouzit, git
In-Reply-To: <20081031190157.GG17651@mit.edu>

Theodore Tso wrote:
> On Fri, Oct 31, 2008 at 09:36:33AM -0700, Jakub Narebski wrote:
>>> +#if 0
>>> +	warning("git revert is deprecated, please use git cherry-pick --revert/-R instead");
>>> +#endif
>> By the way, Mercurial names this command IIRC 'hg backout'. 
> 
> By the way, BitKeeper names this command "bk undo" (which is another
> reason why I would advocate against "git undo" as a syntatic sugar for
> "git checkout HEAD -- $*") --- not that I think there are too many BK
> refugees that might want to switch to git, but it shows that "undo"
> has its own ambiguities; gk uses "undo" the same way we currently use
> "git revert".
> 
> For people who argue that "git cherry-pick --revert" or "git
> cherry-pick -R" is too long, I'd argue that for most people its not a
> common command, and for those for which it is common, they can always
> make in alias for "git pick".
> 

Probably "git unpick" in that case.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply


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