Git development
 help / color / mirror / Atom feed
* Re: [PATCH 5/5] Remove unnecessary 'fetch' argument from transport_get API
From: Shawn O. Pearce @ 2007-09-16  8:11 UTC (permalink / raw)
  To: Junio C Hamano, Daniel Barkalow; +Cc: git
In-Reply-To: <20070915072314.GE20346@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> wrote:
> We don't actually need to know at the time of transport_get if the
> caller wants to fetch, push, or do both on the returned object.
> It is easier to just delay the initialization of the HTTP walker
> until we know we will need it by providing a CURL specific fetch
> function in the curl_transport that makes sure the walker instance
> is initialized before use.

Daniel privately emailed me his rationale for why this fetch argument
was here in the first place and I mostly agree with him, but will
be working up a more clear API replacement in the near future.
The "1/0","fetch/push" thing is not the clearest way that we could
define the API.

Right now this patch is required to go along with my 1/5 bug fix,
as without this patch we get the sequence:

	http_init()
	http_init()
	... use http ...
	http_cleanup()
	... try to use http again and barf ...

This 5/5 makes the sequence be in proper order by delaying creation
of the HTTP walker object (and thus one of the http_init() calls)
to after the first http_cleanup(), so we get the nice neat order of:

	http_init()
	... use http ...
	http_cleanup()
	http_init()
	... use http just fine ...
	http_cleanup()

I'll be honest here; I did not test 1/5 or 5/5 on their own.
I only tested the combined result of them, and that creates a
working HTTP fetch.  But I'm pretty sure that one of these patches
alone will still cause SIGSEGV/SIGBUS errors during HTTP fetch due
to either the bad cleanup (1/5 fixes) or the bad init ordering I'm
talking about above (5/5 fixes).

Reinstating a replacement for the fetch parameter that I removed
in this patch isn't critical for functionality, but will be
necessary to do performance optimization in the form of reusing
the connection between ref discovery and pack transfer in the
native protocol.  Right now I'm focusing on making builtin-fetch
stable and implementing prior behavior.  Once its solid enough to
graduate to `next` we can start doing some of the optimization work.
 
-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] New strbuf APIs: splice and embed.
From: Pierre Habouzit @ 2007-09-16  8:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwsur77g7.fsf@gitster.siamese.dyndns.org>

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

On Sun, Sep 16, 2007 at 12:57:44AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> > * strbuf_splice replace a portion of the buffer with another.
> > * strbuf_embed replace a strbuf buffer with the given one, that should be
> >   malloc'ed. Then it enforces strbuf's invariants. If alloc > len, then this
> >   function has negligible cost, else it will perform a realloc, possibly
> >   with a cost.
> 
> "embed" does not sound quite right, does it?  It is a reverse
> operation of strbuf_detach() as far as I can tell.

  Well I don't like either, and indeed strbuf_attach() seems better.

> > -void strbuf_rtrim(struct strbuf *sb)
> > -{
> > +void strbuf_rtrim(struct strbuf *sb) {
> >  	while (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1]))
> >  		sb->len--;
> >  	sb->buf[sb->len] = '\0';
> 
> This is changing the style in the wrong direction, isn't it?  We
> start our functions like this:
> 
> 	type name(proto)
>         {

  Well that's what I usually do for my code, but I thought git was
putting the opening brace on the same line, my bad.

> > +void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,
> > +				   const void *data, size_t dlen)
> > +{
> > +	if (pos + len < pos)
> > +		die("you want to splice outside from the buffer");
> 
> That is a funny error message for an integer wrap-around check.

  right ;)

> 
> > +	if (pos > sb->len)
> > +		pos = sb->len;
>
> Shouldn't this be flagged as a programming error?
> 
> > +	if (pos + len > sb->len)
> > +		len = sb->len - pos;
> 
> Likewise.

  I just attached the same semantics that was chose for strbuf_insert
when the insertion position is outside from the buffer. Though I can
enforce those to stay inside the buffer and just die(). I don't care
much I shall say. Maybe it hides some programmers being sloppy, hence is
a bad semantics. I'll propose a patch where the check die()s instead of
"fixing" the values.

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

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

^ permalink raw reply

* Re: metastore
From: Junio C Hamano @ 2007-09-16  8:06 UTC (permalink / raw)
  To: david
  Cc: Johannes Schindelin, Daniel Barkalow, martin f krafft, git,
	Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
	David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709151737400.24221@asgard.lang.hm>

david@lang.hm writes:

> git has pre-commit hooks that could be used to gather the permission
> information and store it into a file.
>
> git now has the ability to define cusom merge strategies for specific
> file types, which could be used to handle merges for the permission
> files.
> ...
> There are some significant advantages of having the permission store
> be just a text file.
>
> 1. it doesn't require a special API to a new datastore in git
>
> 2. when working in an environment that doesn't allow for implementing the
>    permissions (either a filesystem that can't store the permissions or
>    when not working as root so that you can't set the ownership) the file
>    can just be written and then edited with normal tools.
>
> 3. normal merge tools do a reasonable job of merging them.
>
> however to do this git would need to gain the ability to say 'this
> filename is special, it must be checked out before any other file is
> checked out' (either on a per-directory or per-repository level)

I'd rather not implement it at such a low level where a true
"checkout" happens.  For one thing, I am afraid that the special
casing will affect the normal codepath too much and would make
it into a maintenance nightmare.  But more importantly, if you
are switching between commits (this includes switching branches,
checking out a different commit to a detached HEAD, or
pulling/merging updates your HEAD and updates your work tree),
and the contents of a path does not change between the original
commit and the switched-to commit, you may still have to
"checkout" the external information for that path if your
"permission information file" are different between these two
commits.  To the underlying checkout aka "two tree merge"
operation, that kind of change is invisible and it should stay
so for performance reasons, not to harm the normal operation.
IOW, I do not want the core level to even know about the
existence of "permission information file", even the code that
implements it is well isolated, ifdefed out or made conditional
based on some config variable.

I however think your idea to have extra "permission information
file" is very interesting.  What would be more palatable, than
mucking with the core level git, would be to have an external
command that takes two tree object names that tells it what the
old and new trees our work tree is switching between, and have
that command to:

 - inspect the diff-tree output to find out what were checked
   out and might need their permission information tweaked;

 - inspect the differences between the "permission information
   file" in these trees to find out what were _not_ checked out,
   but still need their permission information tweaked.

 - tweak whatever external information you are interested in
   expressing in your "permission information file" in the work
   tree for the paths it discovered in the above two steps.
   This step may involve actions specific to projects and call
   hook scripts with <path, info from "permission information
   file" for that path> tuples to carry out the actual tweaking.

If we go that route, I am not deeply opposed to add code to
Porcelains to call that new command after they "checkout" a new
commit at the very end of their processing (namely, git-commit,
git-merge, git-am, and git-rebase).

Yes, I am very well aware that somebody already mentioned "there
is a window between the true checkout and permission tweaking".
If you need to touch the core level in order to close that
window, I am not interested.

> The ability to handle /etc comes up every few months. it's got to be
> the most common unimplemented request git has seen.

Asking a pony for many times does not necessary make it the
right for you to have the pony.  The sane way to implement this
is in your Makefile, as Randal and other people with more
experience have already pointed out, and I happen to agree with
them.

My gut feeling is that the approach to use an external hook that
reads your "permission information file" could be done with
negligible impact to the normal operation of git.  I suspect
that the "new command" I suggested above that would run after
"checkout" actions would perform what people need to do in their
Makefiles' "install" rules (if they have the work tree vs target
tree distinction), or "post-checkout" rules (if they want to use
the work tree in-place), and not having to write/reinvent a
Makefile target for this in every project would hopefully make
it easier to use.  That is the only reason I am writing this
message on this topic.

> so would changes like this be acceptable?

That is a different question.  Is having an extention to help
people who want to manage perm bits a worthy goal?  Perhaps, but
it depends.  Is it worthy enough goal to complicate the really
core parts of the code and add huge maintenance burden?
Absolutely not.  Can it be made in such a way that it does not
have much impact to the core parts?  We need to see how it is
done.

^ permalink raw reply

* Re: [StGit PATCH 00/13] Eliminate 'top' and 'bottom' files
From: Catalin Marinas @ 2007-09-16  7:28 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: David Kågedal, git
In-Reply-To: <20070915234244.GD25507@diana.vm.bytemark.co.uk>

On 16/09/2007, Karl Hasselström <kha@treskal.com> wrote:
> On 2007-09-15 00:31:09 +0200, David Kågedal wrote:
>
> > The following series removes the 'bottom' and 'top' files for each
> > patch, and instead uses the commit objects to keep track of the
> > patches.
>
> Wonderful! Does this ensure that there's a bijection between patches
> and commits at _all_ times, or am I missing something?

We should get rid of top.old and bottom.old as well.

My question - does this conflict with the DAG patches in any way? I
intend to include the them at some point, once I get a chance to test
the performance penalty with a big tree like the Linux kernel.

> Hmm, wait, no. Right. We also have to create commits for those patches
> that don't have exactly one commit object. Not that there'll be many
> of them, but better not make assumptions ...

Is there any patch which consists of more than one commit? Maybe only
uncommit could generate one but I think we put some tests in place.

-- 
Catalin

^ permalink raw reply

* [PATCH 3/3] Remove more debugging from builtin-fetch
From: Shawn O. Pearce @ 2007-09-16  6:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Older git-fetch.sh doesn't print "ref: X" when invoked as
`git fetch $url X" so we shouldn't do that now in the new
builtin version.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 builtin-fetch.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/builtin-fetch.c b/builtin-fetch.c
index d9272ed..20926e0 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -530,8 +530,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		}
 		refs[j] = NULL;
 		ref_nr = j;
-		for (j = 0; refs[j]; j++)
-			printf("ref: %s\n", refs[j]);
 	}
 
 	signal(SIGINT, unlock_pack_on_signal);
-- 
1.5.3.1.1000.g7319b

^ permalink raw reply related

* [PATCH 2/3] Don't configure remote "." to fetch everything to itself
From: Shawn O. Pearce @ 2007-09-16  6:32 UTC (permalink / raw)
  To: Junio C Hamano, Daniel Barkalow; +Cc: git

When we are talking about a remote URI of "." we are really talking
about *this* repository that we are fetching into or pushing out of.
There are no matching tracking branches for this repository; we
do not attempt to map a ref back to ourselves as this would either
create an infinite cycle (for example "fetch = +refs/*:refs/mine/*")
or it causes problems when we attempt to push back to ourselves.

So we really cannot setup a remote like this:

  [remote "."]
    url = .
    fetch = +refs/*:refs/*

In the case of `git push . B:T` to fast-forward branch T to B's
current commit git-send-pack will update branch T to B, assuming that
T is the remote tracking branch for B.  This update is performed
immediately before git-send-pack asks git-receive-pack to perform
the same update, and git-receive-pack then fails because T is not
where git-send-pack told it to expect T to be at.

In the case of `git fetch .` we really should do the same thing as
`git fetch $otherrepo`, that is load .git/FETCH_HEAD with the commit
of HEAD, so that `git pull .` will report "Already up-to-date".
We have always behaved like this before on this insane request and
we should at least continue to behave the same way.  With the above
(bad) remote configuration we were instead getting fetch errors
about funny refs, e.g. "refs/stash".

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 This fixes the "push to yourself" breakage I mentioned yesterday.

 remote.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/remote.c b/remote.c
index df91b2f..73a34c9 100644
--- a/remote.c
+++ b/remote.c
@@ -356,10 +356,6 @@ struct remote *remote_get(const char *name)
 		add_uri(ret, name);
 	if (!ret->uri)
 		return NULL;
-	if (!strcmp(name, ".")) {
-		// we always fetch "refs/*:refs/*", which is trivial
-		add_fetch_refspec(ret, "refs/*:refs/*");
-	}
 	ret->fetch = parse_ref_spec(ret->fetch_refspec_nr, ret->fetch_refspec);
 	ret->push = parse_ref_spec(ret->push_refspec_nr, ret->push_refspec);
 	return ret;
-- 
1.5.3.1.1000.g7319b

^ permalink raw reply related

* [PATCH 1/3] Allow builtin-fetch to work on a detached HEAD
From: Shawn O. Pearce @ 2007-09-16  6:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

If we are running fetch in a repository that has a detached HEAD
then there is no current_branch available.  In such a case any ref
that the fetch might update by definition cannot also be the current
branch so we should always bypass the "don't update HEAD" test.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 builtin-fetch.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/builtin-fetch.c b/builtin-fetch.c
index 300d563..d9272ed 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -151,7 +151,8 @@ static int update_local_ref(struct ref *ref,
 		return 0;
 	}
 
-	if (!strcmp(ref->name, current_branch->name) &&
+	if (current_branch &&
+	    !strcmp(ref->name, current_branch->name) &&
 	    !(update_head_ok || is_bare_repository()) &&
 	    !is_null_sha1(ref->old_sha1)) {
 		/*
-- 
1.5.3.1.1000.g7319b

^ permalink raw reply related

* Re: metastore (was: Track /etc directory using Git)
From: martin f krafft @ 2007-09-16  6:14 UTC (permalink / raw)
  To: git
  Cc: Johannes Schindelin, Daniel Barkalow, Thomas Harning Jr.,
	Francis Moreau, Nicolas Vilz, David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709152310380.28586@racer.site>

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

also sprach Johannes Schindelin <Johannes.Schindelin@gmx.de> [2007.09.16.0014 +0200]:
> While at it, you should invent a fallback what to do when the
> owner is not present on the system you check out on.  And
> a fallback when checking out on a filesystem that does not support
> owners.

Like rsync, git would use numerical UIDs (which are always present)
by default, but could be told to try to map account names.

If the filesystem does not support owners, chown() would not exist.
I actually tend to think of things the other way around: instead of
a fallback when chown() does not work (what would such a fallback be
other than not chown()ing?), it would only try chown() if such
functionality existed.

> And a fallback when a non-root user uses it.

That's easy, Unix already provides you with that "fallback": pack up
/etc in a tar and unpack it as a normal user...

> Oh, and while you're at it (you said that it would be nice not to
> restrict git in any way: "it is a content tracker") support the
> Windows style "Group-or-User-or-something:[FRW]" ACLs.

Provided we find a way to implement this in an extensible manner,
this should not be hard to do. I can't do it since I don't have
access to a Windows machine.

Your statement does catch me off-guard though. Does git now
officially target Windows?

-- 
martin;              (greetings from the heart of the sun.)
  \____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
 
if you find a spelling mistake in the above, you get to keep it.
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: metastore (was: Track /etc directory using Git)
From: martin f krafft @ 2007-09-16  6:08 UTC (permalink / raw)
  To: git
  Cc: Daniel Barkalow, Johannes Schindelin, Thomas Harning Jr.,
	Francis Moreau, Nicolas Vilz, David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709151430040.5298@iabervon.org>

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

also sprach Daniel Barkalow <barkalow@iabervon.org> [2007.09.15.2156 +0200]:
> Configuration options only apply to the local aspects of the repository. 
> That is, when you clone a repository, you don't get the configuration 
> options from it, in general. And changing configuration options on a 
> repository does not have any effect on the content it contains. So 
> configuration options aren't appropriate.

Sure they are. Just like git-commit figures out your email address 
if user.email is missing from git-config, or core.sharedRepository 
or core.umask deal with permissions only when you tell them to, 
you'd have to enable core.track or else git would just do what it
does right now.

> Git doesn't have any way to represent owners or groups, and they
> would need to be represented carefully in order to make sense
> across multiple computers. If you're adding support for
> metadata-as-content (for more than "is this a script?"), you
> should be able to cover all of the common cases of extended stuff,
> like AFS-style ACLs.

Ideally, git should be able to store an open-ended number of
properties for each object, yes.

> And if you want to allow meaningful development with this
> mechanism (as opposed to just archival of a sequence of states of
> a live system), the normal case will be that the metadata beyond
> +x is manipulated by ordinary users in some way other than
> modifying their working directory.

I have no idea what you mean with that.

> So the normal case here will be like working on a filesystem that
> doesn't support symlinks or an executable bit when this is
> important content.

... and yet, we support symlinks and executable files. But anyway,
I really don't understand what you're trying to say.

-- 
martin;              (greetings from the heart of the sun.)
  \____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
 
"ist gott eine erfindung des teufels?"
                                                 - friedrich nietzsche
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: db/fetch-pack topic (was: What's cooking in git.git (topics))
From: Shawn O. Pearce @ 2007-09-16  4:03 UTC (permalink / raw)
  To: git
In-Reply-To: <20070915074759.GA3099@spearce.org>

> "Shawn O. Pearce" <spearce@spearce.org> wrote:
> > Junio C Hamano <gitster@pobox.com> wrote:
> > > * db/fetch-pack (Fri Sep 14 03:31:25 2007 -0400) 22 commits
> > ...
> > > This is Daniel's fetch-pack in C plus fixups from Shawn.
> > > Unfortunately the fixups breaks t3200 ("*** glibc detected ***
> > > fetch: free(): invalid pointer xxx ***"), which I haven't looked
> > > into yet.

With all of my fixups applied I took some performance measurements
today from two of my larger day-job repositories:

  A.git (356 branches):
    Current Fetch             New "Builtin" Fetch
    ------------------        --------------------
    real    3m19.921s         real    0m9.969s
    user    1m58.050s         user    0m1.449s
    sys     2m4.747s          sys     0m4.465s

  B.git (24 branches):
    Current Fetch             New "Builtin" Fetch
    ------------------        --------------------
    real    0m21.250s         real    0m4.735s
    user    0m10.981s         user    0m0.510s
    sys     0m12.387s         sys     0m1.481s

All runs were a no-op fetch over SSH on the LAN.  The system running
git-fetch was a Cygwin/Windows 2000 box and the server was Solaris 9.
Just starting an SSH connection (`ssh solsrv echo hi`) takes me on
average 1 second so a 4.7s no-op fetch on the smaller repository
is very respectable.

3m19s vs. 9s?  I'll take 9s, thankyouverymuch.  Even if I have
more testing and debugging to do.  Especially since the two repos
above are only a sampling of the actual set I have to deal with on
a daily basis.

-- 
Shawn.

^ permalink raw reply

* Re: metastore (was: Track /etc directory using Git)
From: david @ 2007-09-16  3:00 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Daniel Barkalow, martin f krafft, git, Thomas Harning Jr.,
	Francis Moreau, Nicolas Vilz, David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709160348110.28586@racer.site>

On Sun, 16 Sep 2007, Johannes Schindelin wrote:

> On Sat, 15 Sep 2007, david@lang.hm wrote:
>
>> so would changes like this be acceptable?
>
> Would they be acceptable for you?  If so, go ahead.  If not, don't.

frankly, unless I am willing for fork git (which I am not) it matters a 
whole lot less if such a change is acceptable to me then if it is 
acceptable to the maintainers.

if it's not acceptable to the maintainers as a concept then it's not worth 
going to the effort of producing the patches as they will just be 
rejected.

David Lang

^ permalink raw reply

* Re: metastore (was: Track /etc directory using Git)
From: Johannes Schindelin @ 2007-09-16  2:48 UTC (permalink / raw)
  To: david
  Cc: Daniel Barkalow, martin f krafft, git, Thomas Harning Jr.,
	Francis Moreau, Nicolas Vilz, David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709151737400.24221@asgard.lang.hm>

Hi,

On Sat, 15 Sep 2007, david@lang.hm wrote:

> so would changes like this be acceptable?

Would they be acceptable for you?  If so, go ahead.  If not, don't.

Hth,
Dscho

^ permalink raw reply

* Re: [PATCH 05/12] user-manual: rewrite index discussion
From: J. Bruce Fields @ 2007-09-16  2:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbqcbuzgj.fsf@gitster.siamese.dyndns.org>

On Sun, Sep 09, 2007 at 05:38:20PM -0700, Junio C Hamano wrote:
> At the end of first paragraph as a note would be good.  People
> may scan from top to bottom (maybe "Find...") to look for
> "cache" and find the sentence "this chapter is about index but
> that is what you are looking for".  That feels a natural flow.

OK, I've addressed all the comments I've gotten and pushed an updated
version to:

  git://linux-nfs.org/~bfields/git.git maint

Please pull if it looks ready.

--b.

J. Bruce Fields (12):
      user-manual: adjust section levels in "git internals"
      user-manual: move object format details to hacking-git chapter
      user-manual: rename "git internals" to "git concepts"
      user-manual: create new "low-level git operations" chapter
      user-manual: rewrite index discussion
      user-manual: reorder commit, blob, tree discussion
      user-manual: rewrite object database discussion
      user-manual: move packfile and dangling object discussion
      user-manual: fix introduction to packfiles
      user-manual: todo updates and cleanup
      documentation: replace Discussion section by link to user-manual chapter
      core-tutorial: minor cleanup

 Documentation/Makefile          |    2 +-
 Documentation/core-intro.txt    |  592 --------------------------
 Documentation/core-tutorial.txt |   32 +-
 Documentation/git.txt           |   57 +++-
 Documentation/user-manual.txt   |  868 ++++++++++++++++++++++-----------------
 5 files changed, 548 insertions(+), 1003 deletions(-)
 delete mode 100644 Documentation/core-intro.txt

^ permalink raw reply

* Re: [PATCH 11/12] documentation: replace Discussion section by link to user-manual chapter
From: J. Bruce Fields @ 2007-09-16  1:56 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Junio C Hamano, git
In-Reply-To: <46E45F45.3040109@op5.se>

On Sun, Sep 09, 2007 at 11:01:57PM +0200, Andreas Ericsson wrote:
> J. Bruce Fields wrote:
>> +
>> +A git project normally consists of a working directory with a ".git"
>> +subdirectory at the top level.  The .git directory contains, among other
>> +things, a compressed object database representing the complete history
>> +of the project, a set of pointers into that history ("refs")
>
> ... into that history ("refs" - branches and tags)

I slept on it and took another look, and decided you were right; this is
what I have there now:
 
  A git project normally consists of a working directory with a ".git"
  subdirectory at the top level.  The .git directory contains, among
  other things, a compressed object database representing the complete
  history of the project, an "index" file which links that history to
  the current contents of the working tree, and named pointers into that
  history such as tags and branch heads.
 
--b.

^ permalink raw reply

* Re: metastore
From: david @ 2007-09-16  1:49 UTC (permalink / raw)
  To: Randal L. Schwartz
  Cc: Grzegorz Kulewski, martin f krafft, git, Johannes Schindelin,
	Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
	David Härdeman
In-Reply-To: <86r6kzxvnm.fsf@blue.stonehenge.com>

On Sat, 15 Sep 2007, Randal L. Schwartz wrote:

>>>>>> "david" == david  <david@lang.hm> writes:
>
>>> Why does everyone keep wanting "work dir == live dir".  Ugh!  The work dir is
>>> the *source*... it gets *copied* into your live dir *somehow*.  And *that* is
>>> where the meta information needs to be.  In that "somehow".
>
> david> the problem is that at checkin you need to do the reverse process. the
> david> other tools that you use on the system work on the live dir, not the
> david> 'work dir', so it's only a 'work dir' in that git requires it as an
> david> staging step between the repository and the place where it's going to
> david> be used.
>
> Eh?  Are we still talking about a "website", or "/etc"?  I'm talking about the
> website case.  I don't do *anything* to the live site.  When I want to add a
> file, I add it to my dev repo, possibly modifying my Makefile, and then spit
> it out on my staging server.  (You *do* have one of those, right?)  Once I
> know it's good, I push it to the live repo, and then "go live" with it.  I
> *never* work on the files that are the result of "make install".

even when working on a website it can be relavent.

yes, when you are developing html you want to do it on a test server , 
move it to staging, and then move to production. but it's also not 
uncommon to have web based tools that allow other people to make some 
changes as well (for example, a bank's website is mostly maintained by 
their web development company, but the bank administraters want the 
ability to change rate information instantly). sometimes this is 
implemented by writing the info to a database and then querying that 
database for every hit, but a far more efficiant way is to store that data 
in a file on the webserver, which can include modifying pages directly.

but yes, I was mostly thinking of /etc instead of the webserver when I 
wrote that.

David Lang

^ permalink raw reply

* Re: metastore (was: Track /etc directory using Git)
From: david @ 2007-09-16  1:35 UTC (permalink / raw)
  To: Daniel Barkalow
  Cc: martin f krafft, git, Johannes Schindelin, Thomas Harning Jr.,
	Francis Moreau, Nicolas Vilz, David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709151430040.5298@iabervon.org>

On Sat, 15 Sep 2007, Daniel Barkalow wrote:

>> The reason why I say this should be done inside git rather than with 
>> hooks and an external tool, such as metastore is quite simple: git 
>> knows about every content entity in any tree of a repo and already has 
>> a data node for each object. Rather than introducing a parallel object 
>> database (shadow hierarchy or single file), it would make a lot more 
>> sense and be way more robust to attach additional information to these 
>> object nodes, wouldn't it?
>
> Git doesn't have any way to represent owners or groups, and they would 
> need to be represented carefully in order to make sense across multiple 
> computers. If you're adding support for metadata-as-content (for more 
> than "is this a script?"), you should be able to cover all of the common 
> cases of extended stuff, like AFS-style ACLs. And if you want to allow 
> meaningful development with this mechanism (as opposed to just archival 
> of a sequence of states of a live system)

don't underestimate the usefullness of the ability to archive and restore 
snapshots of a live system. just that ability would be wonderful to have.

the ability to checkout a copy of things elsewhere and tinker with it 
would be better, but the lack of that doesn't eliminate the utility by any 
means.

David Lang

> , the normal case will be that 
> the metadata beyond +x is manipulated by ordinary users in some way 
> other than modifying their working directory. So the normal case here 
> will be like working on a filesystem that doesn't support symlinks or an 
> executable bit when this is important content.

^ permalink raw reply

* Re: metastore (was: Track /etc directory using Git)
From: david @ 2007-09-16  1:30 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Daniel Barkalow, martin f krafft, git, Thomas Harning Jr.,
	Francis Moreau, Nicolas Vilz, David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709152310380.28586@racer.site>

On Sat, 15 Sep 2007, Johannes Schindelin wrote:

> On Sat, 15 Sep 2007, Daniel Barkalow wrote:
>
>> Git doesn't have any way to represent owners or groups, and they would
>> need to be represented carefully in order to make sense across multiple
>> computers.
>
> [speaking mostly to the proponents of git-as-a-backup-tool]
>
> While at it, you should invent a fallback what to do when the owner is not
> present on the system you check out on.  And a fallback when checking out
> on a filesystem that does not support owners.
>
> And a fallback when a non-root user uses it.
>
> Oh, and while you're at it (you said that it would be nice not to restrict
> git in any way: "it is a content tracker") support the Windows style
> "Group-or-User-or-something:[FRW]" ACLs.

git has pre-commit hooks that could be used to gather the permission 
information and store it into a file.

git now has the ability to define cusom merge strategies for specific file 
types, which could be used to handle merges for the permission files.

what git lacks the ability to do is to deal with special cases on 
checkout.

the handling of gitattributes came really close, but there are two 
problems remaining.

1. whatever is trying to write the files with the correct permissions
    needs to be able to query the permission store before files are
    written. This needs to either be an API call into git to retreive the
    information for any file when it's written, or the ability to define a
    specific file to be checked out first so that it can be used for
    everything else.

2. the ability to specify a custom routine/program to write the file out
    (assuming that it's being written to a filesystem not a pipe). this
    routine would be responsible for querying the permission store and
    doing 'the right thing' when the file is written during a checkout

there are some significant advantages of having the permission store be 
just a text file.

1. it doesn't require a special API to a new datastore in git

2. when working in an environment that doesn't allow for implementing the
    permissions (either a filesystem that can't store the permissions or
    when not working as root so that you can't set the ownership) the file
    can just be written and then edited with normal tools.

3. normal merge tools do a reasonable job of merging them.

however to do this git would need to gain the ability to say 'this 
filename is special, it must be checked out before any other file is 
checked out' (either on a per-directory or per-repository level)

if this is acceptable then altering the routines that write the files to 
have the additional option of calling a different routine based on the 
settings in .gitattributes seems relativly simple. there should already be 
logic to decect if it's writing to a pipe or a filesystem (it needs to 
know if it should set the write bit if nothing else), and there's the 
existing passthrough or custom routine logic for the crlf translation from 
.gitattributes. combining the logic of the two should handle the output 
issues.

the ability to handle /etc comes up every few months. it's got to be the 
most common unimplemented request git has seen. Adding the nessasary hooks 
for it to be done could end up being less effort then repeatedly telling 
people that they shouldn't use git for that task (or should wrap git in 
their own scripts and use the result instead of useing git directly)


so would changes like this be acceptable?

David Lang

^ permalink raw reply

* Re: metastore
From: Randal L. Schwartz @ 2007-09-16  1:10 UTC (permalink / raw)
  To: david
  Cc: Grzegorz Kulewski, martin f krafft, git, Johannes Schindelin,
	Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
	David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709151733260.24221@asgard.lang.hm>

>>>>> "david" == david  <david@lang.hm> writes:

>> Why does everyone keep wanting "work dir == live dir".  Ugh!  The work dir is
>> the *source*... it gets *copied* into your live dir *somehow*.  And *that* is
>> where the meta information needs to be.  In that "somehow".

david> the problem is that at checkin you need to do the reverse process. the
david> other tools that you use on the system work on the live dir, not the
david> 'work dir', so it's only a 'work dir' in that git requires it as an
david> staging step between the repository and the place where it's going to
david> be used.

Eh?  Are we still talking about a "website", or "/etc"?  I'm talking about the
website case.  I don't do *anything* to the live site.  When I want to add a
file, I add it to my dev repo, possibly modifying my Makefile, and then spit
it out on my staging server.  (You *do* have one of those, right?)  Once I
know it's good, I push it to the live repo, and then "go live" with it.  I
*never* work on the files that are the result of "make install".

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

^ permalink raw reply

* Re: [PATCH] New strbuf APIs: splice and embed.
From: Junio C Hamano @ 2007-09-16  0:57 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git
In-Reply-To: <20070915141335.ECB4B4C152@madism.org>

Pierre Habouzit <madcoder@debian.org> writes:

> * strbuf_splice replace a portion of the buffer with another.
> * strbuf_embed replace a strbuf buffer with the given one, that should be
>   malloc'ed. Then it enforces strbuf's invariants. If alloc > len, then this
>   function has negligible cost, else it will perform a realloc, possibly
>   with a cost.

"embed" does not sound quite right, does it?  It is a reverse
operation of strbuf_detach() as far as I can tell.

> -void strbuf_rtrim(struct strbuf *sb)
> -{
> +void strbuf_rtrim(struct strbuf *sb) {
>  	while (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1]))
>  		sb->len--;
>  	sb->buf[sb->len] = '\0';

This is changing the style in the wrong direction, isn't it?  We
start our functions like this:

	type name(proto)
        {
        	...

> +void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,
> +				   const void *data, size_t dlen)
> +{
> +	if (pos + len < pos)
> +		die("you want to splice outside from the buffer");

That is a funny error message for an integer wrap-around check.

> +	if (pos > sb->len)
> +		pos = sb->len;

Shouldn't this be flagged as a programming error?

> +	if (pos + len > sb->len)
> +		len = sb->len - pos;

Likewise.

By the way, this is the kind of situation I wish everybody wrote
their comparison in textual order.  I had to draw a picture like
this to see what was going on.

        sb->buf
        xxxxxxxxxxxzzzzzxxxxxxxxxxx\0
        ^                         ^
        0                   sb->len
                   ^    ^               ^    ^        
                   pos  pos+len         pos  pos+len  
                   v                    v
                   yyyyyyyyyy           yyyyyyyyyy    
                             ^                    ^   
                             dlen                 dlen
                   -------------        --------------
                   pos < sb->len        sb->len < pos

^ permalink raw reply

* Re: [PATCH] builtin-apply: use strbuf's instead of buffer_desc's.
From: Junio C Hamano @ 2007-09-16  0:56 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git
In-Reply-To: <20070915141340.334CA4C152@madism.org>

Pierre Habouzit <madcoder@debian.org> writes:

>  1 files changed, 73 insertions(+), 130 deletions(-)

Nice reduction.

> -		}
> -		return got != size;
> +
> +		nsize = buf->len;
> +		nbuf = convert_to_git(path, buf->buf, &nsize);
> +		if (nbuf)
> +			strbuf_embed(buf, nbuf, nsize, nsize);
> +		return 0;


I suspect that changing the convert_to_git() interface to work
on strbuf instead of (char*, size_t *) pair might make things
simpler and easier.

^ permalink raw reply

* Re: metastore
From: david @ 2007-09-16  0:37 UTC (permalink / raw)
  To: Randal L. Schwartz
  Cc: Grzegorz Kulewski, martin f krafft, git, Johannes Schindelin,
	Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
	David Härdeman
In-Reply-To: <86veaby050.fsf@blue.stonehenge.com>

On Sat, 15 Sep 2007, Randal L. Schwartz wrote:

>>>>>> "Grzegorz" == Grzegorz Kulewski <kangur@polcom.net> writes:
>
> Grzegorz> Not only for tracking /etc or /home but also for example for "web
> Grzegorz> applications" (for example in PHP). In that case file and directory
> Grzegorz> permissions can be as important as the source code tracked and it is pain to
> Grzegorz> chmod (and sometimes chown) all files to different values after each
> Grzegorz> checkout. Not speaking about potential race.
>
> Uh, works just fine for me to manage my web site content.  The point is
> that I treat git for what it is... a source code management system.
> And then I have a Makefile that "installs" my source code into the live
> directory, with the right modes during installation.
>
> Why does everyone keep wanting "work dir == live dir".  Ugh!  The work dir is
> the *source*... it gets *copied* into your live dir *somehow*.  And *that* is
> where the meta information needs to be.  In that "somehow".

the problem is that at checkin you need to do the reverse process. the 
other tools that you use on the system work on the live dir, not the 'work 
dir', so it's only a 'work dir' in that git requires it as an staging step 
between the repository and the place where it's going to be used.

David Lang

^ permalink raw reply

* Re: Stg AssertionError in sink command
From: Karl Hasselström @ 2007-09-15 23:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Jon Smirl, Git Mailing List
In-Reply-To: <b0943d9e0709151537k51f2f8d5w5a9bbfef84f82747@mail.gmail.com>

On 2007-09-15 23:37:21 +0100, Catalin Marinas wrote:

> The initial implementation of hide/unhide was to preserve the patch
> position in the stack. This behaviour was really confusing for
> people since pushing a patch after the hidden one actually forced
> the pushing of the hidden patch, which isn't normally shown by
> 'series'. I decided afterwards to create a third category of patches
> - 'hidden' (the other two being applied and unapplied). This cleared
> the unpredictable behaviour when pushing since hidden patches cannot
> be pushed.

I agree that the new semantics is way saner (though I'm still not
entirely convinced that hiding patches is a good idea in the first
place). But the man pages should probably point out clearly that
hidden patches must be unapplied.

> Karl's patch does the correct thing of raising an error if sinking
> below an unapplied patch.
>
> Regarding the reordering of the unapplied patches, at the moment you
> can (but I *don't* recommend) edit .git/patches/<branch>/unapplied.
> Maybe the 'float' and 'sink' functionality could be modified to also
> act on unapplied patches (with a --unapplied option). I don't think
> a separate command would be needed as it duplicates a lot of
> functionality from the above.

Why the extra option? Just do the right thing, since it's unambiguous
what the right thing is.

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

^ permalink raw reply

* Re: [StGit PATCH 00/13] Eliminate 'top' and 'bottom' files
From: Karl Hasselström @ 2007-09-15 23:42 UTC (permalink / raw)
  To: David Kågedal; +Cc: git, catalin.marinas
In-Reply-To: <20070914222819.7001.55921.stgit@morpheus.local>

On 2007-09-15 00:31:09 +0200, David Kågedal wrote:

> The following series removes the 'bottom' and 'top' files for each
> patch, and instead uses the commit objects to keep track of the
> patches.

Wonderful! Does this ensure that there's a bijection between patches
and commits at _all_ times, or am I missing something?

> The last two patches do the final cleansing. Obviously, this changes
> the format, and the format version should be increased and and
> update function be written. So it's not really ready to go in yet.

It's a trivial format update, though: just delete those two files and
increase the number from 2 to 3.

Hmm, wait, no. Right. We also have to create commits for those patches
that don't have exactly one commit object. Not that there'll be many
of them, but better not make assumptions ...

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

^ permalink raw reply

* Re: Conflicting "-n" short options for git-pull?
From: Junio C Hamano @ 2007-09-15 23:39 UTC (permalink / raw)
  To: Frans Pop; +Cc: git
In-Reply-To: <200709152114.54985.elendil@planet.nl>

Frans Pop <elendil@planet.nl> writes:

> According to the man page for git-pull from git-core 1.5.3.1 (Debian 
> package), two options are defined as having the short option "-n":
>
>      -n, --no-summary
>          Do not show diffstat at the end of the merge.
> [...]
>      -n, --no-tags
>          By default, git-fetch fetches tags that point at objects that are
>          downloaded from the remote repository and stores them locally. This
>          option disables this automatic tag following.

The manpage option descriptions are shared between the
commands.  Maybe we should drop mention of the shorthand form.

When git-fetch is used -n means --no-tags because there is no
other -n; when git-pull indirectly invokes git-fetch, you need
to spell it --no-tags because --no-summary takes precedence.

^ permalink raw reply

* Re: [StGit PATCH 13/13] Remove the 'top' field
From: Karl Hasselström @ 2007-09-15 23:36 UTC (permalink / raw)
  To: David Kågedal; +Cc: git, catalin.marinas
In-Reply-To: <20070914223215.7001.80066.stgit@morpheus.local>

On 2007-09-15 00:32:15 +0200, David Kågedal wrote:

> @@ -436,7 +422,13 @@ class Series(PatchSet):
>                  patch = patch.strip()
>                  os.rename(os.path.join(branch_dir, patch),
>                            os.path.join(patch_dir, patch))
> -                Patch(patch, patch_dir, refs_base).update_top_ref()
> +                topfield = os.path.join(patch_dir, patch, 'top')
> +                if os.path.isfile(topfield):
> +                    top = read_string(topfield, False)
> +                else:
> +                    top = None
> +                if top:
> +                    git.set_ref(refs_base + '/' + patch, top)
>              set_format_version(1)
>  
>          # Update 1 -> 2.

And remove the top file, maybe? (Or I may be mistaken; I don't have a
copy of the surrounding code handy.)

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

^ 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