* [PATCH 1/7] git-mv: Remove dead code branch
From: Petr Baudis @ 2008-07-16 19:11 UTC (permalink / raw)
To: git
In-Reply-To: <20080716190753.19772.93357.stgit@localhost>
The path list builder had a branch for the case the source is not in index, but
this can happen only if the source was a directory. However, in that case we
have already expanded the list to the directory contents and set mode
to WORKING_DIRECTORY, which is tested earlier.
The patch removes the superfluous branch and adds an assert() instead. git-mv
testsuite still passes.
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
builtin-mv.c | 16 +++++++---------
1 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/builtin-mv.c b/builtin-mv.c
index 5530e11..158a83d 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -227,15 +227,13 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
if (mode == WORKING_DIRECTORY)
continue;
- if (cache_name_pos(src, strlen(src)) >= 0) {
- path_list_insert(src, &deleted);
-
- /* destination can be a directory with 1 file inside */
- if (path_list_has_path(&overwritten, dst))
- path_list_insert(dst, &changed);
- else
- path_list_insert(dst, &added);
- } else
+ assert(cache_name_pos(src, strlen(src)) >= 0);
+
+ path_list_insert(src, &deleted);
+ /* destination can be a directory with 1 file inside */
+ if (path_list_has_path(&overwritten, dst))
+ path_list_insert(dst, &changed);
+ else
path_list_insert(dst, &added);
}
^ permalink raw reply related
* [RFC][PATCH 0/7] Submodule support in git mv, git rm
From: Petr Baudis @ 2008-07-16 19:11 UTC (permalink / raw)
To: git, git
The following series implements submodule support in git mv and git rm,
plus enhancing the submodules testsuite a bit. I'd appreciate comments,
especially on the git mv change, since the index_path_src_sha1 is
really a horrible hack.
The pinnacle of this series was supposed to be merge-recursive support
for submodule-somethingelese conflicts, however that seems a bit more
complicated than I expected, so I decided to first send the rest for
a review.
---
Petr Baudis (7):
t7403: Submodule git mv, git rm testsuite
git rm: Support for removing submodules
git mv: Support moving submodules
submodule.*: Introduce simple C interface for submodule lookup by path
git submodule add: Fix naming clash handling
t7400: Add short "git submodule add" testsuite
git-mv: Remove dead code branch
Documentation/git-rm.txt | 6 +
Makefile | 2
builtin-mv.c | 67 ++++++++++--
builtin-rm.c | 65 ++++++++++--
cache.h | 2
git-submodule.sh | 15 ++-
sha1_file.c | 10 ++
submodule.c | 50 +++++++++
submodule.h | 8 +
t/t7400-submodule-basic.sh | 39 +++++++
t/t7403-submodule-mvrm.sh | 242 ++++++++++++++++++++++++++++++++++++++++++++
11 files changed, 476 insertions(+), 30 deletions(-)
create mode 100644 submodule.c
create mode 100644 submodule.h
create mode 100755 t/t7403-submodule-mvrm.sh
^ permalink raw reply
* Re: Considering teaching plumbing to users harmful
From: Junio C Hamano @ 2008-07-16 19:09 UTC (permalink / raw)
To: Avery Pennarun; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <32541b130807161151x19c20f9t91b7fb9b8c7b8c7b@mail.gmail.com>
"Avery Pennarun" <apenwarr@gmail.com> writes:
> svn avoids these excess merges by default, albeit because it puts your
> working copy at risk every time you do "svn update".
By default? As if it has other mode of operation.
Of course if you do not allow any commits in between to make the history
truly forked, you won't see merge commits. It is like saying that you
like your broken keyboard whose SHIFT key does not work because you think
capital letters look ugly and your keyboard protects you from typing them
by accident.
Is that an improvement?
I won't waste my time further on the apples and rotten oranges comparison,
but you should perhaps listen to Linus's talk where he talks about why it
sucks that SVN/CVS _encourage_ you to keep your local changes uncommitted
for several weeks.
>> You can skip merges with "git log --no-merges", just in case you didn't
>> know.
>
> Perhaps this is mostly a user education or documentation issue. I
> know about --no-merges, but it's unclear that this is really a safe
> thing to use, particularly if some of your merges have conflicts.
> Leaving them out leaves out an important part of history. Do you use
> this option yourself?
Very rarely. When I run "git shortlog" for summary, it often is handy,
but otherwise no.
^ permalink raw reply
* Re: Considering teaching plumbing to users harmful
From: Petr Baudis @ 2008-07-16 18:59 UTC (permalink / raw)
To: Avery Pennarun; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <32541b130807161151x19c20f9t91b7fb9b8c7b8c7b@mail.gmail.com>
On Wed, Jul 16, 2008 at 02:51:30PM -0400, Avery Pennarun wrote:
> On 7/16/08, Junio C Hamano <gitster@pobox.com> wrote:
> > You can skip merges with "git log --no-merges", just in case you didn't
> > know.
>
> Perhaps this is mostly a user education or documentation issue. I
> know about --no-merges, but it's unclear that this is really a safe
> thing to use, particularly if some of your merges have conflicts.
> Leaving them out leaves out an important part of history. Do you use
> this option yourself?
Whereas if you rebase, not only you don't show the conflicts resolution,
you didn't even _store_ it in the first place. That isn't much of an
improvement. :-) (This is the main reason why I prefer to avoid rebase
unless absolutely necessary for the workflow.)
Petr "Pasky" Baudis
^ permalink raw reply
* Re: [PATCH,RFC] Implement 'git rm --if-missing'
From: Peter Baumann @ 2008-07-16 18:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ciaran McCreesh, git
In-Reply-To: <7vtzepr7g5.fsf@gitster.siamese.dyndns.org>
On Wed, Jul 16, 2008 at 11:48:42AM -0700, Junio C Hamano wrote:
> Ciaran McCreesh <ciaran.mccreesh@googlemail.com> writes:
>
> > git rm --if-missing will only remove files if they've already been removed from
> > disk.
>
> This probably is a borderline with feaping creaturism. What's the use of
> it in a real workflow that you need this for?
>
> "git add -u" may be too broad in that it also adds anything modified, but
> so is --if-missing too broad in that it removes anything removed, and if
> you are going to limit by giving pathspecs _anyway_, then...
>
> Old timers might just do:
>
> git diff --name-only --diff-filter=D |
> git update-index --remove --stdin
>
> ;-)
>
Ah. This comes in handy. I already searched for a command to delete all
missing files. After reading through the fine manual of 'git rm', I went
to git update-index but didn't come up with a solution to my problem.
But I have to say, an argument to 'git rm' would be preferable than the
above plumping.
-Peter
^ permalink raw reply
* Re: Considering teaching plumbing to users harmful
From: Avery Pennarun @ 2008-07-16 18:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7v7iblsnfh.fsf@gitster.siamese.dyndns.org>
On 7/16/08, Junio C Hamano <gitster@pobox.com> wrote:
> "Avery Pennarun" <apenwarr@gmail.com> writes:
> > git diff :{1,3}:path/to/filename
> >
> > Which is a great command, but svn definitely makes it easier to do the
> > same thing.
>
> I've never seen anybody who finds "diff :{1,3}:path" *useful*.
Dunno. I use it frequently, and it works great for me. Perhaps my
brain is just poisoned by svn.
I've never tried "git log -p --merge". I'll try it next time. This
is certainly not common knowledge, however. (But to save Dscho the
trouble: git usability in general is not the subject of this thread.)
> > Even if you have a repo with widespread push access, git's log looks
> > annoying compared to svn because of all the merge commits. That's a
> > primary reason why rebase was invented, of course.
>
> Please don't talk nonsense if you do not know history. I invented rebase
> primarily because I wanted to help e-mail based contributors. There is
> nothing about merge avoidance to it.
Sorry, I mixed up git-rerere and git-rebase. From git-rerere's man page:
When your topic branch is long-lived, however, your topic branch would
end up having many such "Merge from master" commits on it, which would
unnecessarily clutter the development history. Readers of the Linux
kernel mailing list may remember that Linus complained about such too
frequent test merges when a subsystem maintainer asked to pull from a
branch full of "useless merges".
Nowadays, I'm pretty sure people use git-rebase to avoid this sort of
problem (or "git pull --rebase" presumably wouldn't have appeared),
but I can now see how git-rebase was not written *for* this problem.
Anyway, my point was that git-rebase (or at least git-rerere and
git-reset) are needed if you want to avoid a lot of merge commits.
And, to relate it back to this thread, git-rebase cannot possibly be
understood without understanding git internals, and git internals are
easiest to understand by learning the plumbing.
svn avoids these excess merges by default, albeit because it puts your
working copy at risk every time you do "svn update".
> You can skip merges with "git log --no-merges", just in case you didn't
> know.
Perhaps this is mostly a user education or documentation issue. I
know about --no-merges, but it's unclear that this is really a safe
thing to use, particularly if some of your merges have conflicts.
Leaving them out leaves out an important part of history. Do you use
this option yourself?
Have fun,
Avery
^ permalink raw reply
* Re: [PATCH,RFC] Implement 'git rm --if-missing'
From: Junio C Hamano @ 2008-07-16 18:48 UTC (permalink / raw)
To: Ciaran McCreesh; +Cc: git
In-Reply-To: <1216231250-21141-1-git-send-email-ciaran.mccreesh@googlemail.com>
Ciaran McCreesh <ciaran.mccreesh@googlemail.com> writes:
> git rm --if-missing will only remove files if they've already been removed from
> disk.
This probably is a borderline with feaping creaturism. What's the use of
it in a real workflow that you need this for?
"git add -u" may be too broad in that it also adds anything modified, but
so is --if-missing too broad in that it removes anything removed, and if
you are going to limit by giving pathspecs _anyway_, then...
Old timers might just do:
git diff --name-only --diff-filter=D |
git update-index --remove --stdin
;-)
> diff --git a/builtin-rm.c b/builtin-rm.c
> index 22c9bd1..4b89705 100644
> --- a/builtin-rm.c
> +++ b/builtin-rm.c
> @@ -125,7 +125,7 @@ static int check_local_mod(unsigned char *head, int index_only)
> static struct lock_file lock_file;
>
> static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;
> -static int ignore_unmatch = 0;
> +static int ignore_unmatch = 0, if_missing = 0;
Not your fault in entirety, but we should drop these " = 0"
initializations for static variables in a clean-up patch.
> static struct option builtin_rm_options[] = {
> OPT__DRY_RUN(&show_only),
> @@ -135,6 +135,7 @@ static struct option builtin_rm_options[] = {
> OPT_BOOLEAN('r', NULL, &recursive, "allow recursive removal"),
> OPT_BOOLEAN( 0 , "ignore-unmatch", &ignore_unmatch,
> "exit with a zero status even if nothing matched"),
> + OPT_BOOLEAN( 0 , "if-missing", &if_missing, "only remove missing files"),
Perhaps the command should error out if some of the named files still
exist in the working tree?
> @@ -168,6 +169,12 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
> struct cache_entry *ce = active_cache[i];
> if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))
> continue;
> + if (if_missing)
> + {
> + struct stat st;
> + if ((lstat(ce->name, &st) == 0) || (errno != ENOENT))
> + continue;
> + }
(1) (Style). Opening brace comes on the same line as "if ()".
(2) (Design). How should this new option interact with --cached mode of
operation?
(3) (Design). Shouldn't "git rm --if-missing" without any pathspec remove
all missing paths from the index?
(4) If lstat fails due to I/O error or something, you do not continue and
add that path you did not get ENOENT for to the kill-list. Is that
desirable?
(5) I wonder if lstat() is enough here.
Consider:
- current commit has "kernel" symlink to "linux-2.6/" directory but
you want to remove kernel and move directory linux-2.6 to it, so:
- you run "rm kernel; mv linux-2.6 kernel"
- then you run "git rm --if-missing -- kernel"
What should the command do?
^ permalink raw reply
* [PATCHv2] Documentation/git-submodule.txt: Add Description section
From: Petr Baudis @ 2008-07-16 18:44 UTC (permalink / raw)
To: gitster; +Cc: git, Heikki Orsila
In-Reply-To: <20080715183705.GD4379@zakalwe.fi>
Figuring out how submodules work conceptually is quite a bumpy
ride for a newcomer; the user manual helps (if one knows to actually
look into it), but the reference documentation should provide good
quick intro as well. This patch attempts to do that, with suggestions
from Heikki Orsila.
Cc: Heikki Orsila <shdl@zakalwe.fi>
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
I have adjusted the description a bit; however, I believe mentioning remotes in
the description would only raise the danger of confusion - I emphasized the
level of separation, though.
Documentation/git-submodule.txt | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 76702a0..87c4ece 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -16,6 +16,28 @@ SYNOPSIS
'git submodule' [--quiet] summary [--summary-limit <n>] [commit] [--] [<path>...]
+DESCRIPTION
+-----------
+Submodules are a special kind of tree entries which refer to a particular tree
+in another repository (living at a given URL). The tree entry describes
+the existence of a submodule with the given name and the exact revision that
+should be used, while the location of the repository is described in the
+`/.gitmodules` file.
+
+When checked out, submodules will maintain their own independent repositories
+within their directories; the only link between the submodule and the "parent
+project" is the tree entry within the parent project mentioned above.
+
+This command will manage the tree entries and contents of the gitmodules file
+for you, as well as inspecting the status of your submodules and updating them.
+When adding a new submodule to the tree, the 'add' subcommand is to be used.
+However, when pulling a tree containing submodules, these will not be checked
+out by default; the 'init' and 'update' subcommands will maintain submodules
+checked out and at appropriate revision in your working tree. You can inspect
+the current status of your submodules using the 'submodule' subcommand and get
+an overview of changes 'update' would perform using the 'summary' subcommand.
+
+
COMMANDS
--------
add::
^ permalink raw reply related
* Re: Considering teaching plumbing to users harmful
From: Avery Pennarun @ 2008-07-16 18:35 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807161902400.8986@racer>
On 7/16/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> And with my way of not even bothering to tell users that "git pull" has a
> default remote and branch, it is easy to tell users about pulling from
> somewhere else:
I agree that this is the best way to teach git pull.
> > At the very least, there will be branches.
>
> Oh. And you have to teach plumbing for that?
In svn, a branch is a revision-controlled directory. In git, a branch
is a "ref". What's a ref? Well, it's a name for a commit. What's a
commit? Well, it's a blob. What's a blob? Err, that's complicated.
What happens when I delete a branch? Well, it's still in the reflog.
What's the reflog? Well, it's the local revision history of each
branch. Local? Why not shared? In svn, the revision history of each
branch is shared, but in git, you don't need to, because...
Even git branches are surprisingly concept heavy, unless your users
ask a lot fewer questions than mine. The really critical question is
why it's so easy to delete a branch in git, and that leads rapidly
into the commit-tree stuff, which is always a spiral into plumbing as
you try to explain the tree of commits.
> > And so you suddenly need to find out about things like
> >
> > git diff :{1,3}:path/to/filename
>
> No. Nobody needed that. All except one user were content with "git
> diff". That one wanted "git diff --ours".
I can't find that option in the git-diff man page.
> I will not even bother to reply to your mentioning rebase, submodules, and
> the "complicated" log due to merges for that very reason: all of this can
> be done, easily, with porcelain.
My point was that the porcelain doesn't even make that stuff easy, and
thus you need to understand fundamental git internal concepts to use
them, and fundamental git internals are easiest to teach using the
plumbing, which doesn't try to hide them.
> > Ironically, the plumbing is the only part of git that isn't supposed to
> > ever change, so it's the most valuable knowledge to have.
>
> Aha. So we changed porcelain recently, in a backwards-incompatible way?
> Now, that is news to me.
There are frequent discussions on this list about changing the output
of various porcelains vs. plumbing. Improving the porcelain output is
useful, because a lot of it right now is mostly accidental (especially
error and progress messages), and to make git easier to use over time,
it will presumably want to be cleaned up.
But if I write a script that uses git and I need to parse the output,
those very useful porcelain changes are backwards incompatible.
The common advice in that case is to only write scripts that use the
plumbing, not the porcelain. That's fine advice, I think. But in
svn, I can write scripts using the "svn" command, because its outputs
never change. Quite unadvanced svn users write shell scripts around
svn, including basic things such as:
svn status | grep ^C
...to list all conflicted files. I don't think a similar script
around "git status" is guaranteed not to break. Perhaps I've
misunderstood though.
Have fun,
Avery
^ permalink raw reply
* Re: Considering teaching plumbing to users harmful
From: Johannes Schindelin @ 2008-07-16 18:27 UTC (permalink / raw)
To: Jesper Eskilson; +Cc: git
In-Reply-To: <487E3BAE.80500@iar.se>
Hi,
On Wed, 16 Jul 2008, Jesper Eskilson wrote:
> Johannes Schindelin wrote:
>
> > On Wed, 16 Jul 2008, Jesper Eskilson wrote:
> >
> > > Johannes Schindelin wrote:
> > >
> > > > Now, it makes me really, really sad that Git has a reputation of
> > > > being complicated, but I regularly hear from _my_ users that they
> > > > do not understand how that came about.
> > >
> > > Well, Git is not the easiest tool on the market to learn. For people
> > > used to centralized systems such as RCS/CVS/Subversion, many
> > > concepts are truly alien. I've recently experienced a transition at
> > > our company from MKS/SI (a RCS derivative) to Subversion, and the
> > > mental gap was for many users HUGE. Had we done the transition from
> > > MKS/SI to Git, I'm sure several user's brains would have exploded.
> > >
> > > From my perspective, the concept I found most difficult to grasp at
> > > the very beginning was how the index worked, and many of the
> > > introductory texts on Git that I looked through only very brielfy
> > > explained the purpose of the index: Why is it there? Why is it
> > > called "index"? How does it fit into a typical workflow? Having a
> > > CVS/Subversion background, it took a while for me to really
> > > assimilate the concept.
> >
> > What does your answer have to do with my mail, i.e. with plumbing?
>
> Nothing, really. I just wanted to comment on your note on Git having a
> reputation being complicated.
Thanks, but you also read my note that my users did not find Git
complicated. And I think it is not because I am _such_ a good instructor.
Ciao,
Dscho
^ permalink raw reply
* Re: Hacks for AIX
From: Linus Torvalds @ 2008-07-16 18:26 UTC (permalink / raw)
To: Chris Cowan; +Cc: git
In-Reply-To: <5855afd30807161057v54ed4112jaea3bc07cebf44d4@mail.gmail.com>
On Wed, 16 Jul 2008, Chris Cowan wrote:
>
> I also saw one instance where the behavior of git-grep was affected by
> the grep selected at build time. I'm not sure if there's other
> instances within the code base, but I'm wondering whether the
> configure script can be changed to do the check for /usr/linux/bin and
> use those versions? I can imagine that similar problems may occur on
> Solaris and HPUX.
The grep selection at compile time is purely a choice between "no external
grep at all" and "whatever external grep is in $PATH".
exec_grep() literally does
..
pid = fork();
if (pid < 0)
return pid;
if (!pid) {
execvp("grep", (char **) argv);
exit(255);
}
..
so you can choose your version of external grep at run-time by just
setting PATH appropriately.
Or you can just decide that you don't want to use any external grep binary
at all, which is the compile-time choice of NO_EXTERNAL_GREP. In that
case, git will do the grep implementation all internally. It can do so,
but then it relies on the regex() library which is often less optimized
than the external grep.
Note the "often". It's possible that the external grep is never worth it,
in which case you should use NO_EXTERNAL_GREP. GNU grep happens to be very
good.
Even with an external grep configured in, you'll end up using the internal
one for the case where you ask for the index information ("--cached") or
when you ask for a particular version of the tree rather than the
checked-out tree. So regardless, you'll fall back to the internal version
for some things.
Linus
^ permalink raw reply
* Re: Hacks for AIX
From: Junio C Hamano @ 2008-07-16 18:25 UTC (permalink / raw)
To: Chris Cowan; +Cc: git
In-Reply-To: <5855afd30807161057v54ed4112jaea3bc07cebf44d4@mail.gmail.com>
"Chris Cowan" <chris.o.cowan@gmail.com> writes:
> * /usr/bin/grep - behaves badly in t7002. I believe it is test
> 12 and related to the -n -w -e combination of options.
Perhaps your version of AIX needs to...
$ make NO_EXTERNAL_GREP=UnfortunatelyYesOnThisAIX
> * /usr/bin/diff - has problems with -u and -U.
> I saw the $GIT_CMP_TEST env var, but this is
> not used everywhere within the test scripts above.
> In some cases, the tests could have been made more portable by using a
> plain "diff" rather than "diff -u", for example.
A patch to the testsuite to replace use of "diff" and "diff -u" that test
the actual output matches expected output with "test_cmp" would be
appreciated.
> * /usr/bin/patch - really old version, doesn't do well with some
> diff formats. I avoid using it.
t4109 seems to use patch to produce expected output for the tests; we
should ship a precomputed expected results. Do you know of any other
places "patch" is used?
^ permalink raw reply
* Re: [PATCH 2/3] add new Git::Repo API
From: Jakub Narebski @ 2008-07-16 18:21 UTC (permalink / raw)
To: Lea Wiemann; +Cc: git, John Hawley, Petr Baudis
In-Reply-To: <1215738665-5153-1-git-send-email-LeWiemann@gmail.com>
[Here is promised patch review]
On Fri, 11 July 2008, Lea Wiemann wrote:
> create mode 100644 perl/Git/Commit.pm
> create mode 100644 perl/Git/Object.pm
> create mode 100644 perl/Git/Repo.pm
> create mode 100644 perl/Git/RepoRoot.pm
> create mode 100644 perl/Git/Tag.pm
Does splitting into many small files is really necessary?
> create mode 100755 t/t9710-perl-git-repo.sh
> create mode 100755 t/t9710/test.pl
Good that you provided test suite.
> diff --git a/perl/Git/Commit.pm b/perl/Git/Commit.pm
> new file mode 100644
> index 0000000..a9bc304
> --- /dev/null
> +++ b/perl/Git/Commit.pm
> @@ -0,0 +1,163 @@
> +=head1 NAME
> +
> +Git::Commit - Object-oriented interface to Git commit objects.
> +
> +=cut
> +
> +use strict;
> +use warnings;
> +
> +
> +package Git::Commit;
> +
> +use base qw(Git::Object);
> +
> +use constant _MESSAGE => 'M';
> +use constant _ENCODING => 'E';
> +use constant _TREE => 'T';
> +use constant _PARENTS => 'P';
> +use constant _AUTHOR => 'A';
> +use constant _COMMITTER => 'C';
What is this for? Why not keep content of commit headers keyed under
their names? It would be much simpler code, and I don't think the
above buys anything in terms of performance.
There is another thing to consider: commit object and tag object format
was deliberately (I think) designed to be *extensible* (in a way).
You can add new headers, and old git binary should simply ignore
unknown headers. This is what allowed us to add 'encoding' header
to commit object, and propose 'keyword' header to tag object (which
was to be used for notes / post-factum annotations).
> +# Keep documentation in one place to save space.
Errrrr... that is a stupid idea, and even more stupid reasoning.
Embedded PODs in Perl modules serve as sort of literate programming,
serving to describe code (technical/usage documentation) in addition
to comments in code.
> +=item $commit = Git::Commit->new($repo, $sha1)
> +
> +Return a new Git::Commit instance for a commit object with $sha1 in
> +repository $repo.
> +
> +Calls to this method are free, since it does not check whether $sha1
> +exists and has the right type. However, accessing any of the commit
> +object's properties will fail if $sha1 is not a valid commit object.
> +
> +Note that $sha1 must be the SHA1 of a commit object; tag objects are
> +not dereferenced.
The technique you use has a name, and it is (IIUC) "lazy evaluation".
By the way, wouldn't it be better to make this method internal, and
use instead the following code to generate Git::Commit object
$commit = $repo->commit($sha1);
> +=item $obj->repo
> +=item $obj->sha1
Those do not access the repository, isn't it?
> +=item $commit->tree
> +
> +Return the tree this commit object refers to.
I think you should say there explicitely that it is SHA-1 of tree
(tree object identifier) that is returned. Isn't it?
By the way I think it should be mentioned explicitly in documentation
which methods cause "instantation" (which cause evaluation).
[The fact that documentation is separated from code means that
I cannot easily tell and write if code match documentation]
> +=item $commit->parents
> +
> +Return a list of zero or more parent commit objects. Note that commit
> +objects stringify to their respective SHA1s, so you can alternatively
> +treat this as a list of SHA1 strings.
Array or arrayref?
There is little inconsistency that tree object is (from the lack of
Git::Tree object) returned as SHA1, and parents as objects.
NOTE that element of list of revisions has in addition to that also
_effective_ parents in the event of history simplification, for example
for 'history' view, or when using '--first-parent' extra option.
> +=item $commit->authors
s/authors/author/; singular.
> +
> +Return the author string of this commit object.
> +
> +=item $commit->committer
> +
> +Return the committer string of this commit object.
It returns whole value of 'author' and 'committer' headers, not
something extracted from it (into name, email, epoch and timezone),
isn't it?
> +=item $commit->message
> +
> +Return the undecoded commit message of this commit object.
Just raw data?
NOTE that for element of list of revisions (as returned by git-rev-list
or git-log) would probably have commit message decoded to UTF-8 by git.
> +=item $commit->encoding
> +
> +Return the encoding header of the commit object.
Normalized?
> +sub tree {
> + my $self = shift;
> + $self->_load;
> + $self->{_TREE()};
> +}
> +
> +sub parents {
> + my $self = shift;
> + $self->_load;
> + map { ref($self)->new($self->repo, $_) } @{$self->{_PARENTS()}};
> +}
Nice... but shouldn't ->tree return Git::Tree?
> +sub author {
> + my $self = shift;
> + $self->_load;
> + $self->{_AUTHOR()} or '';
> +}
> +
> +sub committer {
> + my $self = shift;
> + $self->_load;
> + $self->{_COMMITTER()} or '';
> +}
Nowhere in documentation is mentioned that you use empty value for no
author or no committer (isn't commit object invalid then?).
> +# Auxiliary method to load (and parse) the commit object from the
> +# repository if it hasn't already been loaded.
> +
> +sub _load {
> + my($self, $raw_text) = shift;
Probably should read:
+ my ($self, $raw_text) = shift;
> + return if defined $self->{_MESSAGE()}; # already loaded
Do you ensure that 'message' (or 'body') is always defined? Why not
use "exists" instead; is there some reason to chose "defined"?
> + my $sha1 = $self->sha1;
> + if (!defined $raw_text) {
> + # Retrieve from the repository.
> + (my $type, $raw_text) = $self->repo->cat_file($sha1);
> + die "$sha1 is a $type object (expected a commit object)"
> + unless $type eq 'commit';
> + }
The above makes Git::Commit good solution for gitweb's 'commit' and
'commitdiff' views, but bad solution for 'log', 'shortlog', 'history'
and 'rss'/'atom' views, where you would need to many command
invocations, which is very bad on OS with slow fork.
> + (my $header, $self->{_MESSAGE()}) = split "\n\n", $raw_text, 2;
> + # Parse header.
Why not simply parse headers, then slurp rest of object into 'message'
field? I'm just asking, because it is alternate (but I don't know if
better) solution.
> + for my $line (split "\n", $header) {
> + local $/ = "\n"; # for chomp
> + chomp($line);
> + my($key, $value) = split ' ', $line, 2;
> + if ($key eq 'tree') {
> + $self->{_TREE()} = $value;
> + } elsif ($key eq 'parent') {
> + push @{$self->{_PARENTS()}}, $value;
> + } elsif ($key eq 'author') {
> + $self->{_AUTHOR()} = $value;
> + } elsif ($key eq 'committer') {
> + $self->{_COMMITTER()} = $value;
> + } elsif ($key eq 'encoding') {
> + $self->{_ENCODING()} = $value;
> + } else {
> + die "unrecognized commit header $key";
> + }
IMHO you should not die at unrecognized header, but simply store it
under its name (and make available using ->header('<NAME>')). Please
remember that commit object format is extensible.
Nevertheless I think you can check for header name validation (there
are some restrictions on header names, isn't it?).
> +=head1 NOTES
> +
> +You will usually want to call $repo->get_commit($sha1) instead of
> +instantiating this class directly; see L<Git::Repo>.
> +
> +=cut
Ahhh, O.K.
Why not $repo->commit($sha1)?
> diff --git a/perl/Git/Object.pm b/perl/Git/Object.pm
> new file mode 100644
> index 0000000..a5126f9
> --- /dev/null
> +++ b/perl/Git/Object.pm
> @@ -0,0 +1,81 @@
> +=head1 NAME
> +
> +Git::Object - Object-oriented interface to Git objects (base class).
> +
> +=head1 DESCRIPTION
> +
> +Git::Object is a base class that provides access to commit, tag and
> +(unimplemented) tree objects. See L<Git::Commit> and L<Git::Tag>.
Is it base class which represents types of objects in git repository:
commits, tags, trees and blobs? Or just a class which represent
headers+payload objects, i.e. commits and tags?
> +Objects are loaded lazily, and hence instantiation is free. Objects
> +stringify to their SHA1s.
This should probably be repeated in documentation for Git::Commit and
Git::Tree (possibly linking to Git::Object(3pm) manpage), to be more
self sufficient.
> +use overload
> + '""' => \&stringify;
Nice.
> +# Hash indices:
> +use constant _REPO => 'R';
> +use constant _SHA1 => 'H';
Again: Is it necessary?
> +=head1 METHODS
> +=item Git::Object->new($repo, $sha1)
> +=item $obj->repo
> +=item $obj->sha1
Why those methods are IIRC repeated in Git::Commit, instead of being
inherited?
I wonder if Git::Object should provide $obj->id alias to $obj->sha1...
> diff --git a/perl/Git/Repo.pm b/perl/Git/Repo.pm
> new file mode 100644
> index 0000000..5a4c7e0
> +use strict;
> +use warnings;
> +use 5.006002;
Why is this "use 5.006002" for?
By the way here you (correctly) intersperse code and its documentation,
contrary to some other files in this patch.
> +our @EXPORT = qw();
> +our @EXPORT_OK = qw(assert_sha1 assert_opts);
> +
> +# Auxiliary subroutines
> +
> +sub assert_opts {
> + die "must have an even number of arguments for named options"
> + unless $#_ % 2;
> +}
> +
> +sub assert_sha1 {
> + my $sha1 = shift;
> + die "'$sha1' is not a SHA1 (need to use get_sha1?)"
> + unless $sha1 && $sha1 =~ /^[a-f0-9]{40}$/;
> +}
Hmmm... exporting auxiliary subroutines?
> +=item $repo = Git::Repo->new(%opts)
> +
> +Return a new Git::Repo object. The following options are supported:
> +
> +=over
> +
> +=item 'directory'
> +
> +The directory of the repository (mandatory).
> +
> +Note that this option is working-copy agnostic; you need to
> +instantiate it with the working copy's .git directory as the
> +'directory' option.
Wouldn't it be better to allow the same discovery of '.git' directory
as other git command do, and leave 'git_dir' to set directly path to
repository itself?
> +=item 'git_binary'
> +
> +The name or full path of the git binary (default: 'git').
Probably should be Git::Cmd or Git object, instead. Well,
'git_binary' could set this up indirectly, though...
> +
> +=back
> +
> +Calling this method is free, since it does not check whether the
> +repository exists. Trying to access the repository through one of the
> +instance methods will fail if it doesn't exist though.
> +sub new {
> + my $class = shift;
> + assert_opts(@_);
> + my $self = {@_};
> + bless $self, $class;
> + die 'no directory given' unless $self->{directory};
> + return $self;
> +}
Not even rudimentary check: if directory exists, if it looks like
git repository?
> +
> +=item $repo->repo_dir
> +
> +Return the directory of the repository (.../.git in case of a working
> +copy).
I think $repo->git_dir (perhaps in addition to above) would be better
name, as it is already established among git commands.
> +=item $repo->version
> +
> +Return the output of 'git --version', with /^git version / stripped.
> +
> +This method does not require the repository to exist.
> +
> +=cut
> +
> +sub version{
> + my $self = shift;
> + chomp(my $version = $self->cmd_output(cmd => ['--version']));
> + $version =~ s/^git version //; # be permissive if this does not match
> + return $version;
> +}
We could rely instead on embedded (during build) version string...
or leave _that_ to $Git::Repo::VERSION / $Git::VERSION.
> +=back
> +
> +=head2 Calling the Git binary
> +
> +=over
> +
> +=item $repo->cmd_output(%opts)
Please do remember that there are git commands which do not need
access to git repository, for example git-ls-remote, git-clone,
git-init, even git-config. So tying calling Git binary to
_repository_ instance is a bad API; as a convenience (adding
'--git-dir=<path to repo>' automatically) it is a good idea.
> +Return the output of the given git command as a string, or as a list
> +of lines in array context. Valid options are:
> +
> +=over
> +
> +=item 'cmd'
> +
> +An arrayref of arguments to pass to git (mandatory).
> +
> +=item 'max_exit_code'
> +
> +Die if the exit code of the git binary is greater than
> +C<max_exit_code> (default: 0).
> +
> +=back
> +
> +Example:
> +
> + $output = $repo->cmd_output(cmd => ['show', 'HEAD'])
> +
I think it would be easier on users if you provide two ways of calling
this command: simple and advanced (see for example CGI methods):
$output = $repo->cmd_output('show', 'HEAD');
or
$output = $repo->cmd_output(['show', 'HEAD'],
max_exit_code=>undef);
(which I think should mean: do not die if git command dies).
> +=cut
> +
> +# To do: According to Git.pm, this might not work with ActiveState
> +# Perl on Win 32. Need to check or wait for reports.
Why not copy code from Git.pm, then?
> +=item $repo->get_bidi_pipe(%opts)
> +
> +Open a new bidirectional pipe and return its STDIN and STDOUT file
> +handles. Valid options are:
What about returning context, as it was done in Git.pm?
> +
> +=over
> +
> +=item 'cmd'
> +
> +An arrayref of arguments to pass to git (mandatory).
Same comment as for cmd_output.
> +=item 'reuse'
> +
> +If true, reuse a previously opened pipe with the same command line and
> +whose C<reuse> option was true (default: false).
What is this for? Can you show example usage of this feature? Or is
it a bit of overengineering, perhaps?
You didn't provide equivalents for the rest of command related methods
from Git.pm, even as simple wrappers around other methods:
command_oneline, command_output_pipe, command_input_pipe; you do error
management in different way, so I think you don't need command.*close
methods (note that most of errors, beside not finding git binary and
such, are found on close, not on open).
> +=back
> +
> +=head2 Inspecting the Repository
> +
> +=over
> +
> +=item $repo->get_sha1($extended_object_identifier)
> +
> +Look up the object identified by $extended_object_identifier and
> +return its SHA1 hash in scalar context or its ($sha1, $type, $size) in
> +list context, or undef or () if the lookup failed, where $type is one
> +of 'tag', 'commit', 'tree', or 'blob'.
> +
> +See L<git-rev-parse(1)>, section "Specifying Revisions", for the
> +syntax of the $extended_object_identifier string.
> +
> +Note that even if you pass a SHA1 hash, its existence is still
> +checked, and this method returns undef or () if it doesn't exist in
> +the repository.
I think I'd rather allow extended SHA1 syntax in Git::Commit
and Git::Tag constructors; it is one call to git command less
(I think).
Nevertheless providing equivalent of git-rev-parse (the --revs part)
is, I think, a good idea.
> +=cut
> +
> +sub get_sha1 {
> + my ($self, $object_id) = @_;
> + die 'no object identifier given' unless $object_id;
> + die 'object identifier must not contain newlines' if $object_id =~ /\n/;
> + my ($in, $out) = $self->get_bidi_pipe(
> + cmd => ['cat-file','--batch-check'], reuse => 1);
Ahhh... here I can see what 'reuse => 1' means, and when it is useful.
But doesn't it make sense _only_ for _bi-directional pipe_? Are you
sure that you wouldn't get deadlock?
> +=item $repo->cat_file($sha1)
> +
> +Return the ($type, $content) of the object identified by $sha1, or die
> +if no such object exists in the repository.
> +
> +Note that you may want to use the higher-level methods get_commit and
> +get_tag instead.
Shouldn't it be $repo->get_object($sha1) instead? You don't need to
repeat strange choice of git-cat-file name of command in git, which
is "hysterical raisin"
> +=cut
> +
> +# Possible to-do items: Add optional $file_handle parameter. Guard
> +# against getting huge blobs back when we don't expect it (for
> +# instance, we could limit the size and send SIGPIPE to git if we get
> +# a blob that is too large).
If I remember correctly you do implement something like that (streamed
output) in gitweb patch.
> +=item $repo->get_commit($commit_sha1)
> +=item $repo->get_tag($tag_sha1)
> +
> +Return a new L<Git::Tag> instance referring to the tag object with SHA1
> +$tag_sha1.
Probably should mention lazy evaluation, too.
By the way, for gitweb you would need (for performance and for
rewritten parents) also get_log / get_commits / get_commits_list
> +=item $repo->get_path($tree_sha1, $file_sha1)
> +
> +Return the path of the tree or blob identified by $file_sha1 in the
> +tree identified by $tree_sha1, or undef if the tree or blob does not
> +exist in the given tree.
Note that it is *not* evaluated lazily. Probably should go thru
Git::Tree when it gets implemented.
> +sub get_path {
> + my ($self, $tree, $file_sha1) = @_;
> + assert_sha1($tree, $file_sha1);
> + my @lines = split "\n", $self->cmd_output(cmd => ['ls-tree', '-r', '-t', $tree]);
> + for (@lines) {
> + if (/^[0-9]+ [a-z]+ $file_sha1\t(.+)$/) {
> + return $1;
> + }
> + }
> + return undef;
> +}
You would have troubles with filename quoting! (Probably should be
an option to get_path to get unquoted vs quoted filename).
> +=item $repo->get_refs
> +
> +=item $repo->get_refs($pattern)
> +
> +Return an arrayref of [$sha1, $object_type, $ref_name] triples. If
> +$pattern is given, only refs matching the pattern are returned; see
> +L<git-for-each-ref(1)> for details.
> +sub get_refs {
> + my ($self, $pattern) = @_;
> +
> + return [ map [ split ], split("\n", $self->cmd_output(
> + cmd => [ 'for-each-ref',
> + defined $pattern ? $pattern : () ])) ];
> +}
I think you would need more complete interface to git-for-each-ref
and/or git-show-refs.
By the way, why split("\n", ...) instead of ensuring $\ = "\n" and
(en)forcing list context?
> +=item $repo->name_rev($committish_sha1, $tags_only = 0)
> +
> +Return a symbolic name for the commit identified by $committish_sha1,
> +or undef if no name can be found; see L<git-name-rev(1)> for details.
> +If $tags_only is true, no branch names are used to name the commit.
Why name_rev, and no describe?
> diff --git a/perl/Git/RepoRoot.pm b/perl/Git/RepoRoot.pm
> new file mode 100644
> index 0000000..12e1836
> --- /dev/null
> +++ b/perl/Git/RepoRoot.pm
> @@ -0,0 +1,103 @@
> +=head1 NAME
> +
> +Git::RepoRoot - A factory class representing a root directory
> +containing Git repositories.
> +
> +=head1 DESCRIPTION
> +
> +Git::RepoRoot is a factory class to create L<Git::Repo> instances that
> +are located under a common root directory. It also allows for
> +specifying options that all Git::Repo instances will be created with.
> +
> +Using Git::RepoRoot to create Git::Repo instances is entirely
> +optional, but can be more convenient than instantiating them directly.
Nice. Does Git::RepoRoot provides way to scan for repositories under
commin $projectroot?
> +=over
> +
> +=item 'directory'
> +
> +The directory holding all repositories.
Probably should be named 'root', or 'reporoot', or something...
> +Example:
> +
> + $repo_root = Git::RepoRoot->new(directory => '/pub/git',
> + git_binary => '/usr/bin/git'q);
^
It looks like typo ------------------------------------------------|
> diff --git a/perl/Git/Tag.pm b/perl/Git/Tag.pm
> new file mode 100644
> index 0000000..4e0549b
> +use constant _MESSAGE => 'M';
> +use constant _ENCODING => 'E';
> +use constant _TAGGER => 'A';
> +use constant _TAG => 'T';
> +use constant _TYPE => 'Y';
> +use constant _OBJECT => 'O';
Again.
> +# Keep documentation in one place to save space.
Again.
Tag object format is meant to be extensible; for example (unfortunately
abandoned) git-notes implementation added 'keyword' (IIRC) header to
tag object format.
> +=item $tag->tagger
> +
> +Return the tagger string of this tag object.
We would probably want some way to extract name, email, epoch/date
(and a way to convert epoch+timezone to RFC or ISO format), timezone.
> +=item $tag->message
> +=item $tag->encoding
> +
> +Return the encoding header of the tag object.
Does tag provide 'encoding' header?
Should (for completeness) Git::Tag provide $tag->validate() method?
> +=head1 NOTES
> +
> +You will usually want to call $repo->get_tag($sha1) instead of
> +instantiating this class directly; see L<Git::Repo>.
Good, although this could be put in description of new() instead...
> diff --git a/t/t9710/test.pl b/t/t9710/test.pl
> new file mode 100755
> index 0000000..188abba
> --- /dev/null
> +++ b/t/t9710/test.pl
> @@ -0,0 +1,165 @@
> +#!/usr/bin/perl
> +use lib (split(/:/, $ENV{GITPERLLIB}));
> +
> +use warnings;
> +use strict;
> +
> +use Test::More qw(no_plan);
> +use Test::Exception;
> +use Carp::Always;
Are those necessary? If yes, shouldn't they be checked before running
this test script?
> +our $old_stderr;
> +sub discard_stderr {
> + open our $old_stderr, ">&", STDERR or die "cannot save STDERR";
> + close STDERR;
> +}
> +sub restore_stderr {
> + open STDERR, ">&", $old_stderr or die "cannot restore STDERR";
> +}
Nice.
[I haven't examined test in detail].
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: Considering teaching plumbing to users harmful
From: Jesper Eskilson @ 2008-07-16 18:19 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807161913440.8986@racer>
Johannes Schindelin wrote:
> Hi,
>
> On Wed, 16 Jul 2008, Jesper Eskilson wrote:
>
>> Johannes Schindelin wrote:
>>
>>> Now, it makes me really, really sad that Git has a reputation of being
>>> complicated, but I regularly hear from _my_ users that they do not
>>> understand how that came about.
>> Well, Git is not the easiest tool on the market to learn. For people
>> used to centralized systems such as RCS/CVS/Subversion, many concepts
>> are truly alien. I've recently experienced a transition at our company
>> from MKS/SI (a RCS derivative) to Subversion, and the mental gap was for
>> many users HUGE. Had we done the transition from MKS/SI to Git, I'm sure
>> several user's brains would have exploded.
>>
>> From my perspective, the concept I found most difficult to grasp at the
>> very beginning was how the index worked, and many of the introductory
>> texts on Git that I looked through only very brielfy explained the
>> purpose of the index: Why is it there? Why is it called "index"? How
>> does it fit into a typical workflow? Having a CVS/Subversion background,
>> it took a while for me to really assimilate the concept.
>
> What does your answer have to do with my mail, i.e. with plumbing?
Nothing, really. I just wanted to comment on your note on Git having a
reputation being complicated.
--
/Jesper
^ permalink raw reply
* Re: Considering teaching plumbing to users harmful
From: Junio C Hamano @ 2008-07-16 18:18 UTC (permalink / raw)
To: Avery Pennarun; +Cc: Johannes Schindelin, git
In-Reply-To: <32541b130807161053w24a21d7bh1fa800a714ce75db@mail.gmail.com>
"Avery Pennarun" <apenwarr@gmail.com> writes:
> On 7/16/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> Am I the only one who deems teaching plumbing to users ("I like it raw!
>> So I teach it the same way!") harmful?
>
> I believe the only way you can get away with such a simple learning
> sequence is if your workflow is as simple as that you seem to
> describe: everyone has push access to the central 'master'.
>
> That works (and mostly just as well as any other "supposedly easy"
> VCS, like svn), but because git's power is so tempting, almost
> nobody's real-life repository actually works like that.
>
> At the very least, there will be branches. And where there are
> branches, there's merging. And with merging comes merge conflicts.
Well, you are wrong. Even when people work only with a single branch
'master', once you have more than one repository involved, there's already
merging. Dscho just described how he would guide new people into the
process without going into the details in that message, by the time his
audiences need merge conflict resolution they are already comfortable with
the index.
> git diff :{1,3}:path/to/filename
>
> Which is a great command, but svn definitely makes it easier to do the
> same thing.
I've never seen anybody who finds "diff :{1,3}:path" *useful*.
Well, if you are coming from SVN or CVS where a merge is just a large goo
of everything that happened on a side branch squashed into one, perhaps it
might look useful.
What you should learn and teach instead is:
git log -p --merge
This shows individual changes from the commits involved in the conflict
with rationale (of course your committers must be disciplined enough to
write usable commit log messages for you to take full benefit of this).
Add path/to/filename if you want to process one path at a time. Also
adding --left-right to the command line may make it more understandable if
you are merging two histories, both of which are from other people, and
you do not know which commit is from which side of the merge.
> Even if you have a repo with widespread push access, git's log looks
> annoying compared to svn because of all the merge commits. That's a
> primary reason why rebase was invented, of course.
Please don't talk nonsense if you do not know history. I invented rebase
primarily because I wanted to help e-mail based contributors. There is
nothing about merge avoidance to it.
You can skip merges with "git log --no-merges", just in case you didn't
know.
I won't comment on the remainder but that is not because I agree with
anything you said there ;-)
^ permalink raw reply
* Re: [PATCH,RFC] Implement 'git rm --if-missing'
From: Avery Pennarun @ 2008-07-16 18:17 UTC (permalink / raw)
To: Petr Baudis; +Cc: Ciaran McCreesh, git
In-Reply-To: <20080716180617.GO32184@machine.or.cz>
On 7/16/08, Petr Baudis <pasky@suse.cz> wrote:
> On Wed, Jul 16, 2008 at 07:00:50PM +0100, Ciaran McCreesh wrote:
> > git rm --if-missing will only remove files if they've already been removed from
> > disk.
> >
> > Signed-off-by: Ciaran McCreesh <ciaran.mccreesh@googlemail.com>
>
> what is the usage scenario? The porcelain options space is a precious
> resource, so please explain why do you need this and who is going to use
> it (especially with such a long name).
I see the idea here: right now you can do:
touch a b c
git add .
And have it auto-add all the new files, so "git commit" will work.
But there is no equivalent for rm, because for obvious reasons,
rm b c
git rm .
Doesn't do the same thing. And "git add ." doesn't auto-recognize
deletions, which probably also makes sense.
"git commit -a", on the other hand, will automatically commit all
deletions for you. But you don't always want to commit *all* your
changes just because you want to commit all your deletions.
That said, --if-missing is a bit unwieldy. I don't have a better
suggestion though.
Have fun,
Avery
^ permalink raw reply
* Re: Considering teaching plumbing to users harmful
From: Johannes Schindelin @ 2008-07-16 18:14 UTC (permalink / raw)
To: Jesper Eskilson; +Cc: git
In-Reply-To: <487E34EC.40708@iar.se>
Hi,
On Wed, 16 Jul 2008, Jesper Eskilson wrote:
> Johannes Schindelin wrote:
>
> > Now, it makes me really, really sad that Git has a reputation of being
> > complicated, but I regularly hear from _my_ users that they do not
> > understand how that came about.
>
> Well, Git is not the easiest tool on the market to learn. For people
> used to centralized systems such as RCS/CVS/Subversion, many concepts
> are truly alien. I've recently experienced a transition at our company
> from MKS/SI (a RCS derivative) to Subversion, and the mental gap was for
> many users HUGE. Had we done the transition from MKS/SI to Git, I'm sure
> several user's brains would have exploded.
>
> From my perspective, the concept I found most difficult to grasp at the
> very beginning was how the index worked, and many of the introductory
> texts on Git that I looked through only very brielfy explained the
> purpose of the index: Why is it there? Why is it called "index"? How
> does it fit into a typical workflow? Having a CVS/Subversion background,
> it took a while for me to really assimilate the concept.
What does your answer have to do with my mail, i.e. with plumbing?
Ciao,
Dscho
^ permalink raw reply
* Re: Considering teaching plumbing to users harmful
From: Johannes Schindelin @ 2008-07-16 18:12 UTC (permalink / raw)
To: Avery Pennarun; +Cc: git
In-Reply-To: <32541b130807161053w24a21d7bh1fa800a714ce75db@mail.gmail.com>
Hi,
On Wed, 16 Jul 2008, Avery Pennarun wrote:
> On 7/16/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > Am I the only one who deems teaching plumbing to users ("I like it raw!
> > So I teach it the same way!") harmful?
>
> I believe the only way you can get away with such a simple learning
> sequence is if your workflow is as simple as that you seem to
> describe: everyone has push access to the central 'master'.
That _is_ the most common form.
And with my way of not even bothering to tell users that "git pull" has a
default remote and branch, it is easy to tell users about pulling from
somewhere else:
git pull that.big.machine:~gitte/git my-next
No problem. After having worked with the first form a few time, this
command line is surprisingly easy to teach.
> At the very least, there will be branches.
Oh. And you have to teach plumbing for that?
Besides, you do not start with that. Most users will be happy with one
branch called master for the first day, if not week.
> And where there are branches, there's merging. And with merging comes
> merge conflicts.
Funny that you should say that: I had that case. "git pull origin master"
said something about conflicts. Happily, this user was able to read, and
edited the files mentioned to have conflicts.
After resolving the conflicts (the "<<< === >>>" was known from CVS), add
and commit were again as encountered in the first 2 minutes of my course.
> And so you suddenly need to find out about things like
>
> git diff :{1,3}:path/to/filename
No. Nobody needed that. All except one user were content with "git
diff". That one wanted "git diff --ours".
So there was no use to teach some advanced concepts there, let alone in
the first few lectures.
But back to the subject: what does this have to do with plumbing?
I will not even bother to reply to your mentioning rebase, submodules, and
the "complicated" log due to merges for that very reason: all of this can
be done, easily, with porcelain.
> Ironically, the plumbing is the only part of git that isn't supposed to
> ever change, so it's the most valuable knowledge to have.
Aha. So we changed porcelain recently, in a backwards-incompatible way?
Now, that is news to me.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH,RFC] Implement 'git rm --if-missing'
From: Petr Baudis @ 2008-07-16 18:06 UTC (permalink / raw)
To: Ciaran McCreesh; +Cc: git
In-Reply-To: <1216231250-21141-1-git-send-email-ciaran.mccreesh@googlemail.com>
Hi,
On Wed, Jul 16, 2008 at 07:00:50PM +0100, Ciaran McCreesh wrote:
> git rm --if-missing will only remove files if they've already been removed from
> disk.
>
> Signed-off-by: Ciaran McCreesh <ciaran.mccreesh@googlemail.com>
what is the usage scenario? The porcelain options space is a precious
resource, so please explain why do you need this and who is going to use
it (especially with such a long name).
Petr "Pasky" Baudis
^ permalink raw reply
* [PATCH,RFC] Implement 'git rm --if-missing'
From: Ciaran McCreesh @ 2008-07-16 18:00 UTC (permalink / raw)
To: git; +Cc: Ciaran McCreesh
git rm --if-missing will only remove files if they've already been removed from
disk.
Signed-off-by: Ciaran McCreesh <ciaran.mccreesh@googlemail.com>
---
There's nothing here that can't be done using git update-index, but git rm
is less scary.
Regarding exit status: I'm not sure whether exit status should be based upon
whether any files were actually removed, or whether it should be based upon
whether or not all of the supplied patterns were matched. I've gone for the
latter, so that 'git rm --if-missing -r .' succeeds if there's nothing to
remove.
I'm not sure whether 'missing' is the best word. '--if-noent' might be more
appropriate, but less familiar to some. Or is this worth a short option?
Documentation/git-rm.txt | 8 +++++++-
builtin-rm.c | 9 ++++++++-
t/t3600-rm.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 4d0c495..f9335f3 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -7,7 +7,8 @@ git-rm - Remove files from the working tree and from the index
SYNOPSIS
--------
-'git rm' [-f] [-n] [-r] [--cached] [--ignore-unmatch] [--quiet] [--] <file>...
+'git rm' [-f] [-n] [-r] [--cached] [--ignore-unmatch] [--if-missing]
+ [--quiet] [--] <file>...
DESCRIPTION
-----------
@@ -61,6 +62,11 @@ OPTIONS
--ignore-unmatch::
Exit with a zero status even if no files matched.
+--if-missing::
+ Only remove files if they have been removed from disk. Exit status
+ is still based upon whether matches succeed, not whether a remove
+ actually took place.
+
-q::
--quiet::
'git-rm' normally outputs one line (in the form of an "rm" command)
diff --git a/builtin-rm.c b/builtin-rm.c
index 22c9bd1..4b89705 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -125,7 +125,7 @@ static int check_local_mod(unsigned char *head, int index_only)
static struct lock_file lock_file;
static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;
-static int ignore_unmatch = 0;
+static int ignore_unmatch = 0, if_missing = 0;
static struct option builtin_rm_options[] = {
OPT__DRY_RUN(&show_only),
@@ -135,6 +135,7 @@ static struct option builtin_rm_options[] = {
OPT_BOOLEAN('r', NULL, &recursive, "allow recursive removal"),
OPT_BOOLEAN( 0 , "ignore-unmatch", &ignore_unmatch,
"exit with a zero status even if nothing matched"),
+ OPT_BOOLEAN( 0 , "if-missing", &if_missing, "only remove missing files"),
OPT_END(),
};
@@ -168,6 +169,12 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
struct cache_entry *ce = active_cache[i];
if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))
continue;
+ if (if_missing)
+ {
+ struct stat st;
+ if ((lstat(ce->name, &st) == 0) || (errno != ENOENT))
+ continue;
+ }
add_list(ce->name);
}
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index f542f0a..c7c1810 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -143,6 +143,45 @@ test_expect_success '"rm" command suppressed with --quiet' '
git commit -m "remove file from rm --quiet test"
'
+test_expect_success 'Test that "rm --if-missing" works' '
+ echo frotz > test-file &&
+ echo frotz > other-file &&
+ git add test-file other-file &&
+ git commit -m "add files from rm --if-missing test" &&
+ rm test-file &&
+ git rm --if-missing test-file other-file &&
+ ! git ls-files --error-unmatch test-file &&
+ git ls-files --error-unmatch other-file &&
+ git rm other-file &&
+ git commit -m "remove file from rm --if-missing test"
+'
+
+test_expect_success 'Test that "rm --if-missing -r *" works' '
+ echo frotz > test-file &&
+ mkdir -p frotz &&
+ echo frotz > frotz/other-file &&
+ git add test-file frotz/other-file &&
+ git commit -m "add file from rm --if-missing -r * test" &&
+ rm frotz/other-file
+ git rm --if-missing --ignore-unmatch -r \* &&
+ git ls-files --error-unmatch test-file &&
+ git rm test-file &&
+ git commit -m "remove file from rm --missing -r * test &&
+ ! test -d frotz"
+'
+
+test_expect_success 'Test that "rm --if-missing -r *" works even if nothing is removed' '
+ echo frotz > test-file &&
+ mkdir -p frotz &&
+ echo frotz > frotz/other-file &&
+ git add test-file frotz/other-file &&
+ git commit -m "add file from rm --if-missing -r * test" &&
+ git rm --if-missing --ignore-unmatch -r \* &&
+ git rm test-file frotz/other-file &&
+ git commit -m "remove file from rm --missing -r * test &&
+ ! test -d frotz"
+'
+
# Now, failure cases.
test_expect_success 'Re-add foo and baz' '
git add foo baz &&
@@ -217,4 +256,8 @@ test_expect_success 'Remove nonexistent file returns nonzero exit status' '
! git rm nonexistent
'
+test_expect_success 'Test that "rm --if-missing nonexistent" fails' '
+ ! git rm --if-missing nonexistent
+'
+
test_done
--
1.5.6.3.385.g7c3f1
^ permalink raw reply related
* Hacks for AIX
From: Chris Cowan @ 2008-07-16 17:57 UTC (permalink / raw)
To: git
I saw some earlier postings about this, so I thought I would share my
solution (I'm using xlc, BTW). The following tests were broken for
me (using a pull from 2 days ago).
* t0002-gitfile.sh
* t1002-read-tree-m-u-2way.sh
* t2201-add-update-typechange.sh
* t4109-apply-multifrag.sh
* t4110-apply-scan.sh
* t7002-grep.sh
The problems all seem to be rooted in the default utilities shipped with AIX:
* /usr/bin/grep - behaves badly in t7002. I believe it is test
12 and related to the -n -w -e combination of options.
* /usr/bin/diff - has problems with -u and -U.
I saw the $GIT_CMP_TEST env var, but this is
not used everywhere within the test scripts above.
* /usr/bin/patch - really old version, doesn't do well with some
diff formats. I avoid using it.
* /usr/bin/install - doesn't behave the expected way either.
In some cases, the tests could have been made more portable by using a
plain "diff" rather than "diff -u", for example.
Fortunately, there are optional freeware versions that can be
installed for all of these (along with tar and wish too). These
versions if installed, are all found in /usr/linux/bin (or
equivalently /opt/freeware/bin), I just wish they weren't optional. I
have found that having these utilities installed and prepending
/usr/linux/bin to the PATH results in a clean make test and build.
I also saw one instance where the behavior of git-grep was affected by
the grep selected at build time. I'm not sure if there's other
instances within the code base, but I'm wondering whether the
configure script can be changed to do the check for /usr/linux/bin and
use those versions? I can imagine that similar problems may occur on
Solaris and HPUX.
Otherwise, I'm quite happy with git.
--
CC
^ permalink raw reply
* Re: Considering teaching plumbing to users harmful
From: Avery Pennarun @ 2008-07-16 17:53 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807161804400.8950@racer>
On 7/16/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Am I the only one who deems teaching plumbing to users ("I like it raw!
> So I teach it the same way!") harmful?
I believe the only way you can get away with such a simple learning
sequence is if your workflow is as simple as that you seem to
describe: everyone has push access to the central 'master'.
That works (and mostly just as well as any other "supposedly easy"
VCS, like svn), but because git's power is so tempting, almost
nobody's real-life repository actually works like that.
At the very least, there will be branches. And where there are
branches, there's merging. And with merging comes merge conflicts.
And with merge conflicts comes the crazy (yes, very useful, but still
crazy) conflicted index features. And so you suddenly need to find
out about things like
git diff :{1,3}:path/to/filename
Which is a great command, but svn definitely makes it easier to do the
same thing.
Even if you have a repo with widespread push access, git's log looks
annoying compared to svn because of all the merge commits. That's a
primary reason why rebase was invented, of course. But teaching
people about rebase vs. merge is highly nontrivial. "git pull
--rebase" helps a little, but it's still nontrivial, particularly when
local patch #3 of 5 has a conflict.
Also, inevitably, someone will ask "what happened to those simple svn
revision numbers?" or "when I do a merge, why are the patches from
branch #1 interspersed with the ones from branch #2 in git log?" The
answers are "look at gitk to see the real merge history, that's way
more powerful than svn, and check out git-bisect!" and "use git log
--topo-order" respectively, but those are pretty nontrivial answers
too.
Submodules, which are critical to large-scale software development,
are also very complicated. You can't explain how to use them without
knowing about .git/config, the difference between that and
.gitmodules, the concept of gitlinks (and therefore the concepts of
trees and blobs), the idea of a "disconnected HEAD" (which all
submodules check out by default), the idea that pushing submodules in
the wrong order can create references to non-existing commitids, and
so on. In contrast, the lame svn:externals mechanism is far easier to
explain.
The "problem" with learning git is that it's so powerful. A person
can feel like they've "learned all the svn there is to learn" in a
couple of days, because it really doesn't do all that much. But with
git, if you want to make it *appear* simple, you have to artificially
restrict what you tell people, and because the git *developers* don't
work using that restricted subset of commands, the abstraction always
leaks.
Example: "git remote" didn't originally even have an "rm" subcommand.
Why? Because real git developers knew they could delete a remote by
editing .git/config, and it never even occurred to anyone to do it any
other way. I still do it by editing the file, because the file is in
a nice format and it's still easier than typing "git remote".
The svn developers have the same annoyingly small subset of commands
that their users do. It means svn is much less powerful, but it also
means that subset is actually enough to somehow handle *all* the tasks
a user will run into. After all, there's no other way.
That said, it's debatable if all this is actually a problem. If I
wanted simple-minded, I'd use svn. Ironically, the plumbing is the
only part of git that isn't supposed to ever change, so it's the most
valuable knowledge to have. Why *not* teach it?
Have fun,
Avery
^ permalink raw reply
* Re: [PATCH 0/5] add pack index v2 reading capability to git v1.4.4.4
From: Linus Torvalds @ 2008-07-16 17:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nicolas Pitre, git
In-Reply-To: <7vhcapsqoa.fsf@gitster.siamese.dyndns.org>
On Wed, 16 Jul 2008, Junio C Hamano wrote:
>
> I do not think it should SEGV. The pack-idx signature was chosen rather
> carefully to allow older ones to die gracefully.
Well, Pasky reported differently.
> error: non-monotonic index
> error: Could not read 4a588075c54cd5902e5f4d43b9d6b0c31d0f9769
Pasky's report was
error: non-monotonic index
/usr/bin/git-fetch: line 297: 30402 Segmentation fault git-http-fetch -v -a "$head" "$remote/"
but maybe that was something specific to his case.
Linus
^ permalink raw reply
* Considering teaching plumbing to users harmful
From: Johannes Schindelin @ 2008-07-16 17:21 UTC (permalink / raw)
To: git
Hi,
there have been a number of occasions where I came across people trying to
be helpful and teaching Git newbies a few tricks.
However, in quite a number of cases, which seem to surge over the last
weeks, I see people suggesting the use of rev-parse, ls-tree, rev-list
etc.
Their rationale is invariably "but I found it useful", and they seem to be
unable to recognize the puzzlement in the faces of the people they are
trying to help.
Instead they insist that they did nothing wrong.
I had the pleasure of introducing Git to a few users in the last months
and in my opinion, restricting myself to teaching them these commands
first helped tremendously:
- clone, pull, status, add, commit, push, log
All of these were presented without options, to keep things simple.
In particular, I refrained from giving them the "-a" option to commit.
That seemed to help incredibly with their embracing the index as a natural
concept (which it is).
Often I presented the "pull" and "push" commands _only_ with "origin
master" ("origin is where the repository came from, and master is the
branch; you will want to use other parameters here after you used Git for
a while").
_After_ they grew comfortable with Git, I taught them a few options here
and there, not hiding, but also not promoting the full range of options.
So the next tricks were
- log -p, rm, diff, diff --cached, show
The last one is "show", and with that command, I taught the
"<commit>:" and "<commit>:<file>" syntax, too (which some Git old-timers
did not know about ;-)
The pace needed to be adjusted to the users, in my experience, but not the
order.
Now, it makes me really, really sad that Git has a reputation of being
complicated, but I regularly hear from _my_ users that they do not
understand how that came about.
Am I the only one who deems teaching plumbing to users ("I like it raw!
So I teach it the same way!") harmful?
Ciao,
Dscho "who is sad"
^ permalink raw reply
* Re: [PATCH 0/5] add pack index v2 reading capability to git v1.4.4.4
From: Junio C Hamano @ 2008-07-16 17:08 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Nicolas Pitre, git
In-Reply-To: <alpine.LFD.1.10.0807160924340.2835@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Actually, it fixes a crash. Didn't 1.4.4 SIGSEGV without this on pack-v2?
I do not think it should SEGV. The pack-idx signature was chosen rather
carefully to allow older ones to die gracefully.
error: non-monotonic index
error: Could not read 4a588075c54cd5902e5f4d43b9d6b0c31d0f9769
But as I said in the other message, it's tagged and pushed out already.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox