* Re: Considering teaching plumbing to users harmful
From: Avery Pennarun @ 2008-07-16 19:22 UTC (permalink / raw)
To: Petr Baudis; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <20080716185930.GP32184@machine.or.cz>
On 7/16/08, Petr Baudis <pasky@suse.cz> wrote:
> 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.)
The key here is that I'd expect "git log -p" with a rebased merge at
least shows me the actual changes that are in my repository. "git log
--no-merges" will actually omit things.
Avery
^ permalink raw reply
* Re: [PATCH] Rename ".dotest/" to ".git/rebase" and ".dotest-merge" to "rebase-merge"
From: Junio C Hamano @ 2008-07-16 19:18 UTC (permalink / raw)
To: Theodore Tso, Nanako Shiraishi
Cc: Johannes Schindelin, René Scharfe, gitster, Stephan Beyer,
Joe Fiorini, git, Jari Aalto
In-Reply-To: <20080716012619.GM8185@mit.edu>
Theodore Tso <tytso@mit.edu> writes:
> While you have "git am" open, how about adding an "git am --abort"
> which nukes the .dotest aka .git/rebase directory, and resets HEAD
> back to the original position?
This does not seem to have reached the list nor its archives. I cannot
say I have really looked at it deeply but it may be a good starting
point. It needs docs ;-)
-- >8 --
From: Nanako Shiraishi <nanako3@lavabit.com>
Date: Wed, 16 Jul 2008 19:39:10 +0900
Subject: [PATCH] git am --abort
After failing to apply patches in the middle of a series, "git am --abort"
lets you go back to the original commit.
Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
---
git-am.sh | 19 +++++++++++++++----
t/t4151-am-abort.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 63 insertions(+), 4 deletions(-)
create mode 100755 t/t4151-am-abort.sh
diff --git a/git-am.sh b/git-am.sh
index cc8787b..a44bd7a 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -22,6 +22,7 @@ p= pass it through git-apply
resolvemsg= override error message when patch failure occurs
r,resolved to be used after a patch failure
skip skip the current patch
+abort abandon patch application and clear .dotest directory
rebasing (internal use for git-rebase)"
. git-sh-setup
@@ -120,7 +121,7 @@ It does not apply to blobs recorded in its index."
prec=4
dotest="$GIT_DIR/rebase"
-sign= utf8=t keep= skip= interactive= resolved= binary= rebasing=
+sign= utf8=t keep= skip= interactive= resolved= binary= rebasing= abort=
resolvemsg= resume=
git_apply_opt=
@@ -145,6 +146,8 @@ do
resolved=t ;;
--skip)
skip=t ;;
+ --abort)
+ abort=t ;;
--rebasing)
rebasing=t threeway=t keep=t binary=t ;;
-d|--dotest)
@@ -177,7 +180,7 @@ fi
if test -d "$dotest"
then
- case "$#,$skip$resolved" in
+ case "$#,$skip$resolved$abort" in
0,*t*)
# Explicit resume command and we do not have file, so
# we are happy.
@@ -197,9 +200,17 @@ then
esac ||
die "previous rebase directory $dotest still exists but mbox given."
resume=yes
+
+ case "$abort" in
+ t)
+ rm -fr "$dotest" &&
+ git read-tree -m -u ORIG_HEAD &&
+ git reset ORIG_HEAD && :
+ exit ;;
+ esac
else
- # Make sure we are not given --skip nor --resolved
- test ",$skip,$resolved," = ,,, ||
+ # Make sure we are not given --skip, --resolved, nor --abort
+ test "$skip$resolved$abort" = "" ||
die "Resolve operation not in progress, we are not resuming."
# Start afresh.
diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
new file mode 100755
index 0000000..96b2cd5
--- /dev/null
+++ b/t/t4151-am-abort.sh
@@ -0,0 +1,48 @@
+#!/bin/sh
+
+test_description='am --abort'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+ for i in a b c d e f g
+ do
+ echo $i
+ done >file-1 &&
+ cp file-1 file-2 &&
+ test_tick &&
+ git add file-1 file-2 &&
+ git commit -m initial &&
+ git tag initial &&
+ for i in 2 3 4 5
+ do
+ echo $i >>file-1 &&
+ test_tick &&
+ git commit -a -m $i || break
+ done &&
+ git format-patch initial &&
+ git checkout -b side initial &&
+ echo local change >file-2 &&
+ cp file-2 file-2-expect
+'
+
+test_expect_success 'am stops at a patch that does not apply' '
+ test_must_fail git am 000[124]-*.patch &&
+ git log --pretty=tformat:%s >actual &&
+ for i in 3 2 initial
+ do
+ echo $i
+ done >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'am --abort goes back' '
+ git am --abort &&
+ git rev-parse HEAD >actual &&
+ git rev-parse initial >expect &&
+ test_cmp expect actual &&
+ test_cmp file-2-expect file-2 &&
+ git diff-index --exit-code --cached HEAD
+'
+
+test_done
--
1.5.6
^ permalink raw reply related
* Re: [PATCHv2] Documentation/git-submodule.txt: Add Description section
From: Kalle Olavi Niemitalo @ 2008-07-16 19:15 UTC (permalink / raw)
To: git
In-Reply-To: <20080716184248.6524.38463.stgit@localhost>
Petr Baudis <pasky@suse.cz> writes:
> +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.
I was surprised to learn that a commit ID in a tree does not
prevent Git from pruning the corresponding commit object if
one happens to exist in the same repository. That might be
best documented under "Tree Object" in user-manual.txt though,
rather than in git-submodule.txt.
^ permalink raw reply
* [PATCH 7/7] t7403: Submodule git mv, git rm testsuite
From: Petr Baudis @ 2008-07-16 19:11 UTC (permalink / raw)
To: git
In-Reply-To: <20080716190753.19772.93357.stgit@localhost>
The testsuite for newly added submodule support in git mv, git rm.
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
t/t7403-submodule-mvrm.sh | 242 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 242 insertions(+), 0 deletions(-)
create mode 100755 t/t7403-submodule-mvrm.sh
diff --git a/t/t7403-submodule-mvrm.sh b/t/t7403-submodule-mvrm.sh
new file mode 100755
index 0000000..9b50d6a
--- /dev/null
+++ b/t/t7403-submodule-mvrm.sh
@@ -0,0 +1,242 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Johannes Schindelin
+#
+
+test_description='Test submodules support in git mv and git rm'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+ (mkdir sub-repo &&
+ cd sub-repo &&
+ git init &&
+ echo file > file &&
+ git add file &&
+ git commit -m "sub initial") &&
+ (cp -r sub-repo sub2-repo &&
+ cd sub2-repo &&
+ echo file2 > file &&
+ git add file &&
+ git commit -m "sub commit2") &&
+ git submodule add "$(pwd)/sub-repo" sub &&
+ git submodule add "$(pwd)/sub2-repo" sub2 &&
+ git commit -m initial &&
+ test "$(git config -f .gitmodules submodule.sub.path)" = "sub" &&
+ test "$(git config -f .gitmodules submodule.sub2.path)" = "sub2"
+
+'
+
+test_expect_success 'git mv of a submodule' '
+
+ git mv sub sub.moved &&
+ ! test -d sub &&
+ test -d sub.moved/.git &&
+ ! git ls-files --error-unmatch sub &&
+ test "$(git ls-files --stage --error-unmatch sub.moved | cut -d " " -f 1)" = 160000 &&
+ test "$(git config -f .gitmodules submodule.sub.path)" = "sub.moved" &&
+ ! git config -f .gitmodules submodule.sub.moved.path
+
+'
+
+test_expect_success 'git submodule add vs. git mv' '
+
+ ! git submodule add "$(pwd)/sub2-repo" sub.moved &&
+ git submodule add "$(pwd)/sub2-repo" sub &&
+ test -d sub/.git &&
+ test "$(git config -f .gitmodules submodule.sub.url)" = "$(pwd)/sub-repo" &&
+ test "$(git config -f .gitmodules submodule.sub.path)" = "sub.moved" &&
+ test "$(git config -f .gitmodules submodule.sub~.path)" = "sub"
+
+'
+
+test_expect_success 'git mv onto existing file' '
+
+ echo file > file &&
+ git add file &&
+ ! git mv sub.moved file &&
+ test -d sub.moved &&
+ ! test -d file/.git &&
+ test "$(git ls-files --stage --error-unmatch file | cut -d " " -f 1)" = 100644 &&
+ test "$(git ls-files --stage --error-unmatch sub.moved | cut -d " " -f 1)" = 160000 &&
+ test "$(git config -f .gitmodules submodule.sub.path)" = "sub.moved"
+
+'
+
+test_expect_success 'git mv onto existing directory' '
+
+ mkdir -p dir &&
+ echo file > dir/file &&
+ git add dir/file &&
+ git mv sub.moved dir &&
+ ! test -d sub.moved &&
+ test -d dir/sub.moved/.git &&
+ ! git ls-files --error-unmatch sub.moved &&
+ test "$(git ls-files --stage --error-unmatch dir/sub.moved | cut -d " " -f 1)" = 160000 &&
+ test "$(git config -f .gitmodules submodule.sub.path)" = "dir/sub.moved" &&
+ git mv dir/sub.moved . &&
+ test "$(git config -f .gitmodules submodule.sub.path)" = "sub.moved"
+
+'
+
+test_expect_success 'git mv onto existing submodule' '
+
+ ! git mv sub.moved sub2 &&
+ test -d sub.moved/.git &&
+ ! test -d sub2/sub.moved &&
+ test "$(git ls-files --stage --error-unmatch sub2 | cut -d " " -f 1)" = 160000 &&
+ test "$(git ls-files --stage --error-unmatch sub.moved | cut -d " " -f 1)" = 160000 &&
+ test "$(git config -f .gitmodules submodule.sub.path)" = "sub.moved"
+
+'
+
+test_expect_success 'git mv of multiple submodules' '
+
+ mkdir -p dir &&
+ git mv sub.moved sub dir &&
+ ! test -d sub.moved &&
+ ! test -d sub &&
+ test -d dir/sub.moved/.git &&
+ test -d dir/sub/.git &&
+ ! git ls-files --error-unmatch sub.moved sub &&
+ test "$(git ls-files --stage --error-unmatch dir/sub.moved dir/sub | cut -d " " -f 1 | uniq)" = 160000 &&
+ ! git config -f .gitmodules submodule.dir.path &&
+ test "$(git config -f .gitmodules submodule.sub.path)" = "dir/sub.moved" &&
+ test "$(git config -f .gitmodules submodule.sub~.path)" = "dir/sub"
+
+'
+
+test_expect_success 'git mv of multiple submodules back from a subdir' '
+
+ (cd dir && git mv sub.moved sub .. && cd ..) &&
+ test -d sub.moved &&
+ test -d sub &&
+ ! test -d dir/sub.moved/.git &&
+ ! test -d dir/sub/.git &&
+ ! git ls-files --error-unmatch dir/sub.moved dir/sub &&
+ test "$(git ls-files --stage --error-unmatch sub.moved sub | cut -d " " -f 1 | uniq)" = 160000 &&
+ test "$(git config -f .gitmodules submodule.sub.path)" = "sub.moved" &&
+ test "$(git config -f .gitmodules submodule.sub~.path)" = "sub"
+
+'
+
+test_expect_success 'git mv of non-checked-out submodules' '
+
+ git clone . clone &&
+ (cd clone &&
+ test -d sub &&
+ test -d sub2 &&
+ ! test -d sub/.git &&
+ ! test -d sub2/.git &&
+ git ls-files --stage --error-unmatch sub sub2 > ls-files.out &&
+ mkdir -p dir &&
+ git mv sub sub2 dir &&
+ ! test -d sub &&
+ ! test -d sub2 &&
+ test -d dir/sub &&
+ test -d dir/sub2 &&
+ ! git ls-files --error-unmatch sub sub2 &&
+ test "$(git ls-files --stage --error-unmatch dir/sub dir/sub2 | cut -d " " -f 1 | uniq)" = 160000 &&
+ git ls-files --stage --error-unmatch dir/sub dir/sub2 | sed "s#dir/##g" | diff - ls-files.out &&
+ test "$(git config -f .gitmodules submodule.sub.path)" = "dir/sub" &&
+ test "$(git config -f .gitmodules submodule.sub2.path)" = "dir/sub2" &&
+ (cd dir && git mv sub2 .. && cd ..) &&
+ test -d sub2 &&
+ ! test -d dir/sub2 &&
+ ! git ls-files --error-unmatch dir/sub2 &&
+ test "$(git ls-files --stage --error-unmatch sub2 | cut -d " " -f 1)" = 160000 &&
+ test "$(git config -f .gitmodules submodule.sub2.path)" = "sub2")
+
+'
+
+test_expect_success 'checkpointing state with git commit' '
+
+ git commit -m"checkpoint" -a &&
+ (cd clone && git commit -m"clone checkpoint" -a)
+
+'
+
+test_expect_success 'git rm of a regular submodule' '
+
+ git rm sub2 &&
+ test -d sub2/.git &&
+ ! git ls-files --error-unmatch sub2 &&
+ ! git config -f .gitmodules submodule.sub2.path &&
+ ! git config -f .gitmodules submodule.sub2.url
+
+'
+
+test_expect_success 'git rm of a submodule with name different from path' '
+
+ git rm sub.moved &&
+ test -d sub.moved/.git &&
+ ! git ls-files --error-unmatch sub.moved &&
+ ! git config -f .gitmodules submodule.sub.path &&
+ ! git config -f .gitmodules submodule.sub.url
+
+'
+
+test_expect_success 'git rm of a modified submodule' '
+
+ git mv sub dir/sub && # more fun with richer path
+ (cd dir/sub &&
+ echo mod > file &&
+ git commit -m "sub mod" file) &&
+ git add dir/sub &&
+ ! git rm dir/sub &&
+ test -d dir/sub/.git &&
+ test "$(git ls-files --stage --error-unmatch dir/sub | cut -d " " -f 1)" = "160000" &&
+ git config -f .gitmodules submodule.sub~.path &&
+ git config -f .gitmodules submodule.sub~.url &&
+ git rm -f dir/sub &&
+ test -d dir/sub/.git &&
+ ! git ls-files --error-unmatch dir/sub &&
+ ! git config -f .gitmodules submodule.sub~.path &&
+ ! git config -f .gitmodules submodule.sub~.url
+
+'
+
+test_expect_success 'git rm of a submodule from within a subdirectory' '
+
+ git submodule add "$(pwd)/sub-repo" sub-torm &&
+ mkdir -p dir &&
+ # -f since we did not commit the submodule
+ (cd dir && git rm -f ../sub-torm && cd ..) &&
+ test -d sub-torm/.git &&
+ ! git ls-files --error-unmatch sub-torm &&
+ ! git config -f .gitmodules submodule.sub-torm.path &&
+ ! git config -f .gitmodules submodule.sub-torm.url
+
+'
+
+test_expect_success 'git rm of a non-checked-out submodule' '
+
+ (cd clone &&
+ test -d dir/sub &&
+ ! test -d dir/sub/.git &&
+ git rm dir/sub &&
+ ! test -d dir/sub &&
+ ! git ls-files --error-unmatch dir/sub &&
+ ! git config -f .gitmodules submodule.sub.path &&
+ ! git config -f .gitmodules submodule.sub.url)
+
+'
+
+test_expect_success 'git rm of a non-checked-out submodule w/ different working tree' '
+
+ (cd clone &&
+ rmdir sub2 &&
+ echo cunning > sub2 &&
+ ! git rm sub2 &&
+ test -f sub2 &&
+ test "$(git ls-files --stage --error-unmatch sub2 | cut -d " " -f 1)" = "160000" &&
+ git rm -f sub2 &&
+ ! test -e sub2 &&
+ ! git ls-files --error-unmatch sub2 &&
+ ! git config -f .gitmodules submodule.sub2.path &&
+ ! git config -f .gitmodules submodule.sub2.url)
+
+'
+
+test_done
^ permalink raw reply related
* [PATCH 6/7] git rm: Support for removing submodules
From: Petr Baudis @ 2008-07-16 19:11 UTC (permalink / raw)
To: git
In-Reply-To: <20080716190753.19772.93357.stgit@localhost>
This patch adds support for removing submodules to 'git rm', including
removing the appropriate sections from the .gitmodules file to reflect this
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
Documentation/git-rm.txt | 6 +++-
builtin-rm.c | 65 ++++++++++++++++++++++++++++++++++++++--------
2 files changed, 58 insertions(+), 13 deletions(-)
diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 4d0c495..bfc3dfa 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -20,7 +20,8 @@ and no updates to their contents can be staged in the index,
though that default behavior can be overridden with the `-f` option.
When '--cached' is given, the staged content has to
match either the tip of the branch or the file on disk,
-allowing the file to be removed from just the index.
+allowing the file to be removed from just the index;
+this is always the case when removing submodules.
OPTIONS
@@ -56,7 +57,8 @@ OPTIONS
--cached::
Use this option to unstage and remove paths only from the index.
Working tree files, whether modified or not, will be
- left alone.
+ left alone. Note that this is always assumed when removing
+ a checked-out submodule.
--ignore-unmatch::
Exit with a zero status even if no files matched.
diff --git a/builtin-rm.c b/builtin-rm.c
index 22c9bd1..363d1fa 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -9,6 +9,7 @@
#include "cache-tree.h"
#include "tree-walk.h"
#include "parse-options.h"
+#include "submodule.h"
static const char * const builtin_rm_usage[] = {
"git-rm [options] [--] <file>...",
@@ -17,16 +18,21 @@ static const char * const builtin_rm_usage[] = {
static struct {
int nr, alloc;
- const char **name;
+ struct {
+ const char *name;
+ int is_gitlink;
+ } *info;
} list;
-static void add_list(const char *name)
+static void add_list(const char *name, int is_gitlink)
{
if (list.nr >= list.alloc) {
list.alloc = alloc_nr(list.alloc);
- list.name = xrealloc(list.name, list.alloc * sizeof(const char *));
+ list.info = xrealloc(list.info, list.alloc * sizeof(*list.info));
}
- list.name[list.nr++] = name;
+ list.info[list.nr].name = name;
+ list.info[list.nr].is_gitlink = is_gitlink;
+ list.nr++;
}
static int remove_file(const char *name)
@@ -38,6 +44,13 @@ static int remove_file(const char *name)
if (ret && errno == ENOENT)
/* The user has removed it from the filesystem by hand */
ret = errno = 0;
+ if (ret && errno == EISDIR) {
+ /* This is a gitlink entry; try to remove at least the
+ * directory if the submodule is not checked out; we always
+ * leave the checked out ones as they are */
+ if (!rmdir(name) || errno == ENOTEMPTY)
+ ret = errno = 0;
+ }
if (!ret && (slash = strrchr(name, '/'))) {
char *n = xstrdup(name);
@@ -65,7 +78,7 @@ static int check_local_mod(unsigned char *head, int index_only)
struct stat st;
int pos;
struct cache_entry *ce;
- const char *name = list.name[i];
+ const char *name = list.info[i].name;
unsigned char sha1[20];
unsigned mode;
int local_changes = 0;
@@ -83,7 +96,7 @@ static int check_local_mod(unsigned char *head, int index_only)
/* It already vanished from the working tree */
continue;
}
- else if (S_ISDIR(st.st_mode)) {
+ else if (S_ISDIR(st.st_mode) && !S_ISGITLINK(ce->ce_mode)) {
/* if a file was removed and it is now a
* directory, that is the same as ENOENT as
* far as git is concerned; we do not track
@@ -122,6 +135,22 @@ static int check_local_mod(unsigned char *head, int index_only)
return errs;
}
+static void remove_submodule(const char *name)
+{
+ char *key = submodule_by_path(name);
+ char *sectend = strrchr(key, '.');
+
+ assert(sectend);
+ *sectend = 0;
+
+ config_exclusive_filename = ".gitmodules";
+ if (git_config_rename_section(key, NULL) <= 0)
+ die("cannot remove section `%s' from .gitmodules", key);
+ config_exclusive_filename = NULL;
+
+ free(key);
+}
+
static struct lock_file lock_file;
static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;
@@ -140,7 +169,7 @@ static struct option builtin_rm_options[] = {
int cmd_rm(int argc, const char **argv, const char *prefix)
{
- int i, newfd;
+ int i, newfd, subs;
const char **pathspec;
char *seen;
@@ -168,7 +197,7 @@ 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;
- add_list(ce->name);
+ add_list(ce->name, S_ISGITLINK(ce->ce_mode));
}
if (pathspec) {
@@ -216,9 +245,11 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
* the index unless all of them succeed.
*/
for (i = 0; i < list.nr; i++) {
- const char *path = list.name[i];
+ const char *path = list.info[i].name;
if (!quiet)
- printf("rm '%s'\n", path);
+ printf("rm%s '%s'\n",
+ list.info[i].is_gitlink ? "dir" : "",
+ path);
if (remove_file_from_cache(path))
die("git-rm: unable to remove %s", path);
@@ -238,7 +269,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
if (!index_only) {
int removed = 0;
for (i = 0; i < list.nr; i++) {
- const char *path = list.name[i];
+ const char *path = list.info[i].name;
if (!remove_file(path)) {
removed = 1;
continue;
@@ -248,6 +279,18 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
}
}
+ /*
+ * Get rid of stale submodule setup.
+ */
+ subs = 0;
+ for (i = 0; i < list.nr; i++)
+ if (list.info[i].is_gitlink) {
+ remove_submodule(list.info[i].name);
+ subs++;
+ }
+ if (subs && add_file_to_cache(".gitmodules", 0))
+ die("cannot add new .gitmodules to the index");
+
if (active_cache_changed) {
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(&lock_file))
^ permalink raw reply related
* [PATCH 4/7] submodule.*: Introduce simple C interface for submodule lookup by path
From: Petr Baudis @ 2008-07-16 19:11 UTC (permalink / raw)
To: git
In-Reply-To: <20080716190753.19772.93357.stgit@localhost>
The interface will be used for git-mv and git-rm submodule support.
So far, only the submodule_by_path() function is defined, however more
can be probably expected in the future if/when the git-submodule command
is ported from shell.
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
Makefile | 2 ++
submodule.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
submodule.h | 8 ++++++++
3 files changed, 60 insertions(+), 0 deletions(-)
create mode 100644 submodule.c
create mode 100644 submodule.h
diff --git a/Makefile b/Makefile
index 9b52071..742b5cb 100644
--- a/Makefile
+++ b/Makefile
@@ -368,6 +368,7 @@ LIB_H += run-command.h
LIB_H += sha1-lookup.h
LIB_H += sideband.h
LIB_H += strbuf.h
+LIB_H += submodule.h
LIB_H += tag.h
LIB_H += transport.h
LIB_H += tree.h
@@ -458,6 +459,7 @@ LIB_OBJS += sha1_name.o
LIB_OBJS += shallow.o
LIB_OBJS += sideband.o
LIB_OBJS += strbuf.o
+LIB_OBJS += submodule.o
LIB_OBJS += symlinks.o
LIB_OBJS += tag.o
LIB_OBJS += trace.o
diff --git a/submodule.c b/submodule.c
new file mode 100644
index 0000000..2883ae6
--- /dev/null
+++ b/submodule.c
@@ -0,0 +1,50 @@
+#include "cache.h"
+#include "submodule.h"
+
+
+struct gitmodules_info {
+ const char *path;
+ char *key;
+};
+
+static int gitmodules_worker(const char *key, const char *value, void *info_)
+{
+ struct gitmodules_info *info = info_;
+ const char *subkey;
+
+ if (prefixcmp(key, "submodule."))
+ return 0;
+
+ subkey = strrchr(key, '.');
+ if (!subkey)
+ return 0;
+
+ if (strcmp(subkey, ".path"))
+ return 0;
+
+ if (strcmp(value, info->path))
+ return 0;
+
+ /* Found the key to change. */
+ if (info->key) {
+ error("multiple submodules live at path `%s'", info->path);
+ /* The last one is supposed to win. */
+ free(info->key);
+ }
+ info->key = xstrdup(key);
+ return 0;
+}
+
+char *submodule_by_path(const char *path)
+{
+ struct gitmodules_info info = { path, NULL };
+
+ config_exclusive_filename = ".gitmodules";
+ if (git_config(gitmodules_worker, &info))
+ die("cannot process .gitmodules");
+ if (!info.key)
+ die("the submodule of `%s' not found in .gitmodules", path);
+ config_exclusive_filename = NULL;
+
+ return info.key;
+}
diff --git a/submodule.h b/submodule.h
new file mode 100644
index 0000000..bc74fa0
--- /dev/null
+++ b/submodule.h
@@ -0,0 +1,8 @@
+#ifndef SUBMODULE_H
+#define SUBMODULE_H
+
+/* Find submodule living at given path in .gitmodules and return the key
+ * of its path config variable (dynamically allocated). */
+extern char *submodule_by_path(const char *path);
+
+#endif
^ permalink raw reply related
* [PATCH 5/7] git mv: Support moving submodules
From: Petr Baudis @ 2008-07-16 19:11 UTC (permalink / raw)
To: git
In-Reply-To: <20080716190753.19772.93357.stgit@localhost>
This patch adds support for moving submodules to 'git mv', including
rewriting of the .gitmodules file to reflect the movement.
The usage of struct path_list here is a bit abusive, but keeps the
code simple and hopefully still reasonably readable. The horrid
index_path_src_sha1 hack is unfortunately much worse, however the author
is currently unaware of any more reasonable solution of the problem.
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
builtin-mv.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++------
cache.h | 2 ++
sha1_file.c | 10 ++++++++++
3 files changed, 61 insertions(+), 6 deletions(-)
diff --git a/builtin-mv.c b/builtin-mv.c
index 158a83d..30c4e7d 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -9,6 +9,7 @@
#include "cache-tree.h"
#include "path-list.h"
#include "parse-options.h"
+#include "submodule.h"
static const char * const builtin_mv_usage[] = {
"git-mv [options] <source>... <destination>",
@@ -60,6 +61,24 @@ static const char *add_slash(const char *path)
return path;
}
+static int ce_is_gitlink(int i)
+{
+ return i < 0 ? 0 : S_ISGITLINK(active_cache[i]->ce_mode);
+}
+
+static void rename_submodule(struct path_list_item *i)
+{
+ char *key = submodule_by_path(i->path);
+
+ config_exclusive_filename = ".gitmodules";
+ if (git_config_set(key, (const char *) i->util))
+ die("cannot update .gitmodules");
+ config_exclusive_filename = NULL;
+
+ free(key);
+}
+
+
static struct lock_file lock_file;
int cmd_mv(int argc, const char **argv, const char *prefix)
@@ -77,9 +96,12 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
struct stat st;
struct path_list overwritten = {NULL, 0, 0, 0};
struct path_list src_for_dst = {NULL, 0, 0, 0};
+ /* .util contains sha1 for submodules */
struct path_list added = {NULL, 0, 0, 0};
struct path_list deleted = {NULL, 0, 0, 0};
struct path_list changed = {NULL, 0, 0, 0};
+ /* .path is source path, .util is destination path */
+ struct path_list submodules = {NULL, 0, 0, 0};
git_config(git_default_config, NULL);
@@ -99,7 +121,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
/* special case: "." was normalized to "" */
destination = copy_pathspec(dest_path[0], argv, argc, 1);
else if (!lstat(dest_path[0], &st) &&
- S_ISDIR(st.st_mode)) {
+ S_ISDIR(st.st_mode) &&
+ !ce_is_gitlink(cache_name_pos(dest_path[0], strlen(dest_path[0])))) {
dest_path[0] = add_slash(dest_path[0]);
destination = copy_pathspec(dest_path[0], argv, argc, 1);
} else {
@@ -111,7 +134,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
/* Checking */
for (i = 0; i < argc; i++) {
const char *src = source[i], *dst = destination[i];
- int length, src_is_dir;
+ int length, src_is_dir, src_cache_pos;
const char *bad = NULL;
if (show_only)
@@ -126,7 +149,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
} else if ((src_is_dir = S_ISDIR(st.st_mode))
&& lstat(dst, &st) == 0)
bad = "cannot move directory over file";
- else if (src_is_dir) {
+ else if ((src_cache_pos = cache_name_pos(src, length)) < 0 && src_is_dir) {
const char *src_w_slash = add_slash(src);
int len_w_slash = length + 1;
int first, last;
@@ -193,7 +216,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
} else
bad = "Cannot overwrite";
}
- } else if (cache_name_pos(src, length) < 0)
+ } else if (src_cache_pos < 0)
bad = "not under version control";
else if (path_list_has_path(&src_for_dst, dst))
bad = "multiple sources for the same target";
@@ -218,6 +241,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
for (i = 0; i < argc; i++) {
const char *src = source[i], *dst = destination[i];
enum update_mode mode = modes[i];
+ struct path_list_item *last = NULL;
+ int j;
if (show_only || verbose)
printf("Renaming %s to %s\n", src, dst);
if (!show_only && mode != INDEX &&
@@ -227,14 +252,24 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
if (mode == WORKING_DIRECTORY)
continue;
- assert(cache_name_pos(src, strlen(src)) >= 0);
+ j = cache_name_pos(src, strlen(src));
+ assert(j >= 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);
+ last = path_list_insert(dst, &added);
+ if (ce_is_gitlink(j)) {
+ path_list_insert(src, &submodules)->util = (void *) dst;
+ /* We will note the original HEAD sha1; the submodule
+ * may not be checked out, in which case we would have
+ * no way to re-obtain it later when adding the new
+ * entry. */
+ assert(last);
+ last->util = active_cache[j]->sha1;
+ }
}
if (show_only) {
@@ -254,13 +289,21 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
for (i = 0; i < added.nr; i++) {
const char *path = added.items[i].path;
+ index_path_src_sha1 = added.items[i].util;
if (add_file_to_cache(path, verbose ? ADD_CACHE_VERBOSE : 0))
die("updating index entries failed");
+ index_path_src_sha1 = NULL;
}
for (i = 0; i < deleted.nr; i++)
remove_file_from_cache(deleted.items[i].path);
+ for (i = 0; i < submodules.nr; i++)
+ rename_submodule(&submodules.items[i]);
+
+ if (submodules.nr > 0 && add_file_to_cache(".gitmodules", 0))
+ die("cannot add new .gitmodules to the index");
+
if (active_cache_changed) {
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(&lock_file))
diff --git a/cache.h b/cache.h
index a779d92..998b5fb 100644
--- a/cache.h
+++ b/cache.h
@@ -387,6 +387,8 @@ extern int ce_same_name(struct cache_entry *a, struct cache_entry *b);
extern int ie_match_stat(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
extern int ie_modified(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
+extern unsigned char *index_path_src_sha1;
+
extern int ce_path_match(const struct cache_entry *ce, const char **pathspec);
extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object, enum object_type type, const char *path);
extern int index_pipe(unsigned char *sha1, int fd, const char *type, int write_object);
diff --git a/sha1_file.c b/sha1_file.c
index e281c14..4da6048 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2416,12 +2416,22 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object,
return ret;
}
+unsigned char *index_path_src_sha1;
+
int index_path(unsigned char *sha1, const char *path, struct stat *st, int write_object)
{
int fd;
char *target;
size_t len;
+ if (index_path_src_sha1) {
+ /* SHA1 for this path was prepared by the caller specially;
+ * e.g. comes from original cache entry in some cases of
+ * a rename. */
+ memcpy(sha1, index_path_src_sha1, 20);
+ return 0;
+ }
+
switch (st->st_mode & S_IFMT) {
case S_IFREG:
fd = open(path, O_RDONLY);
^ permalink raw reply related
* [PATCH 3/7] git submodule add: Fix naming clash handling
From: Petr Baudis @ 2008-07-16 19:11 UTC (permalink / raw)
To: git
In-Reply-To: <20080716190753.19772.93357.stgit@localhost>
This patch fixes git submodule add behaviour when we add submodule
living at a same path as logical name of existing submodule. This
can happen e.g. in case the user git mv's the previous submodule away
and then git submodule add's another under the same name.
A test-case is obviously included.
This is not completely satisfactory since .git/config cross-commit
conflicts can still occur. A question is whether this is worth
handling, maybe it would be worth adding some kind of randomization
of the autogenerated submodule name, e.g. appending $$ or a timestamp.
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
git-submodule.sh | 15 ++++++++++++---
t/t7400-submodule-basic.sh | 11 +++++++++++
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 9228f56..a93547b 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -192,10 +192,19 @@ cmd_add()
git add "$path" ||
die "Failed to add submodule '$path'"
- git config -f .gitmodules submodule."$path".path "$path" &&
- git config -f .gitmodules submodule."$path".url "$repo" &&
+ name="$path"
+ if git config -f .gitmodules submodule."$name".path; then
+ name="$path~"; i=1;
+ while git config -f .gitmodules submodule."$name".path; do
+ name="$path~$i"
+ i=$((i+1))
+ done
+ fi
+
+ git config -f .gitmodules submodule."$name".path "$path" &&
+ git config -f .gitmodules submodule."$name".url "$repo" &&
git add .gitmodules ||
- die "Failed to register submodule '$path'"
+ die "Failed to register submodule '$path' (name '$name')"
}
#
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index ab5eb1e..092dffc 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -235,4 +235,15 @@ test_expect_success 'submodule add -b' '
'
+test_expect_success 'submodule add auto-naming clash' '
+
+ git submodule add "$(pwd)/init2/.git" example &&
+ test -d example/.git &&
+ [ "$(git config -f .gitmodules submodule.example.url)" = "$(pwd)/init2" ] &&
+ [ "$(git config -f .gitmodules submodule.example.path)" = "init" ]
+ [ "$(git config -f .gitmodules submodule.example~.url)" = "$(pwd)/init2/.git" ] &&
+ [ "$(git config -f .gitmodules submodule.example~.path)" = "example" ]
+
+'
+
test_done
^ permalink raw reply related
* [PATCH 2/7] t7400: Add short "git submodule add" testsuite
From: Petr Baudis @ 2008-07-16 19:11 UTC (permalink / raw)
To: git
In-Reply-To: <20080716190753.19772.93357.stgit@localhost>
This patch introduces basic tests for
git submodule add
covering the basic functionality and the -b parameter.
A trivial update --init test fix freeloads on this commit as well.
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
t/t7400-submodule-basic.sh | 28 +++++++++++++++++++++++++++-
1 files changed, 27 insertions(+), 1 deletions(-)
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 6c7b902..ab5eb1e 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -200,7 +200,7 @@ test_expect_success 'update --init' '
mv init init2 &&
git config -f .gitmodules submodule.example.url "$(pwd)/init2" &&
- git config --remove-section submodule.example
+ git config --remove-section submodule.example &&
git submodule update init > update.out &&
grep "not initialized" update.out &&
test ! -d init/.git &&
@@ -209,4 +209,30 @@ test_expect_success 'update --init' '
'
+test_expect_success 'submodule add' '
+
+ git submodule add "$(pwd)/init2" init-added &&
+ test -d init-added/.git &&
+ [ "$(git config -f .gitmodules submodule.init-added.url)" = "$(pwd)/init2" ] &&
+ [ "$(git config -f .gitmodules submodule.init-added.path)" = "init-added" ]
+
+'
+
+test_expect_success 'submodule add -b' '
+
+ (
+ cd init2 &&
+ git checkout -b branch &&
+ echo t >s &&
+ git add s &&
+ git commit -m "change branch" &&
+ git checkout master
+ ) &&
+ git submodule add -b branch -- "$(pwd)/init2" init-added-b &&
+ test -d init-added-b/.git &&
+ [ "$(git config -f .gitmodules submodule.init-added-b.url)" = "$(pwd)/init2" ] &&
+ [ "$(cd init2 && git rev-parse branch)" = "$(cd init-added-b && git rev-parse HEAD)" ]
+
+'
+
test_done
^ permalink raw reply related
* [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
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