* Usage of isspace and friends
From: Morten Welinder @ 2005-10-12 1:40 UTC (permalink / raw)
To: GIT Mailing List
Someone needs to audit the usage of isspace, tolower, and friends. There are
things like this in the code:
static int is_dev_null(const char *str)
{
return !memcmp("/dev/null", str, 9) && isspace(str[9]);
}
Since str[9] is of type char it should not be used as a argument to
isspace directly,
but rather be cast to unsigned char:
... isspace((unsigned char)str[9]);
Admittedly that is ugly. Blame K&R. (Glibc has a partial workaround for this
kind of coding bug. On the up side you won't get a crash, but on the down
side you can get the wrong result.)
Morten
^ permalink raw reply
* Re: [PATCH] Fix packname hash generation.
From: Junio C Hamano @ 2005-10-13 2:46 UTC (permalink / raw)
To: git
In-Reply-To: <7vslv6b86l.fsf_-_@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> This changes the generation of hash packfiles have in their names, from
> "hash of object names as fed to us" to "hash of object names in the
> resulting pack, in the order they appear in the index file". The new
> "git-index-pack" command is taught to output the computed hash value
> to its standard output.
In case it was not obvious, this is not a backward incompatible
change. Your existing packs will be valid after this change.
What those 40-byte hashes were buying us was that we did not
have to worry about name clashes. We could have said "these two
packs have the same name so they must have the same set of
objects", but there is no tool that relies on this fact. We
could not even say "these two packs have different names so the
set of objects contained by them must be different" -- the
resulting pack name depended on the order of objects fed to
git-pack-objects, even if you fed the same set of objects.
The really core part never cared about how packfiles and their
indices are named. The only restrictions were that they live
immediately under .git/objects/pack/, have .pack and .idx suffix
respectively, and their basename match with each other.
The commit walkers (anything that link with fetch.c) impose
another limitation that their basenames are "pack-" followed by
40-byte hexadecimal digits. But they do not check if the name
is consistent with the set of objects in the pack (checking it
was computationally infeasible for huge packs in the previous
hashing mechanism -- you have to feed all permutations of
objects contained in the pack to SHA1 hash and see if any
produces the same hash as the pack name). We _could_ now do
this additional check if we wanted to (the same goes to the
really core part in sha1_file.c::check_packed_git_idx()).
In short, it does not matter if your existing packs are named
using the old hashing mechanism. They will continue to be
valid.
But if you really care about consistency, here is an easy way to
rename your existing packs to their new names the new hashing
scheme would produce.
#!/bin/sh
: ${GIT_DIR=.git}
: ${GIT_OBJECT_DIRECTORY="${GIT_DIR}/objects"}
O="$GIT_OBJECT_DIRECTORY"
P="$GIT_OBJECT_DIRECTORY/pack"
for existing in `cd "$GIT_OBJECT_DIRECTORY" &&
find pack -name '*.pack' -print`
do
idx=`expr "$existing" : '\(.*\)\.pack$'`.idx &&
test -f "$O/$idx" || {
echo >&2 "Missing idx $idx?"
continue
}
new=`git-index-pack -o tmp-idx "$O/$existing"` || {
echo >&2 "Corrupt pack $existing?"
continue
}
# index generated for an existing pack should match.
cmp "$O/$idx" tmp-idx || {
echo >&2 "Corrupt idx $idx?"
continue
}
if test "pack/pack-$new.pack" = "$existing"
then
echo >&2 "Already converted $existing."
continue
fi
if test -f "$P/pack-$new.pack" || test -f "$P/pack-$new.idx"
then
echo >&2 "Name clash! $new"
continue
fi
mv "$O/$existing" "$P/pack-$new.pack" &&
mv "$O/$idx" "$P/pack-$new.idx" || {
echo >&2 "Cannot move $existing to $new"
continue
}
echo >&2 "Renamed $existing -> $new"
done
^ permalink raw reply
* [PATCH] clone-pack: new option --keep to keep the pack unexploded.
From: Junio C Hamano @ 2005-10-13 1:23 UTC (permalink / raw)
To: git
In-Reply-To: <7vslv6b86l.fsf_-_@assigned-by-dhcp.cox.net>
With new option --keep, or a configuration item clone.keeppack (we
need a better name, or start allowing dash,"clone.keep-pack"), the packed
data downloaded while cloning is saved as a pack in .git/objects/pack/
locally, with index generated for it with git-index-pack.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
* Here is how to use it.
$ mkdir test
$ cd test
$ git-init-db
defaulting to local storage area
$ git-clone-pack --keep ../other/repo/sito/ry/.git
Packing 9044 objects
$ git-checkout
After cloning, you would notice there is no individual
objects; instead you will find a single packfile in
.git/objects/pack/ directory.
Documentation/git-clone-pack.txt | 7 ++++++-
clone-pack.c | 3 ++-
2 files changed, 8 insertions(+), 2 deletions(-)
applies-to: fa5213875bff7fdb8c7d05f35a047eedf3cb3af2
b64a46c81d8347e59285584e830e1ad99ad387e0
diff --git a/Documentation/git-clone-pack.txt b/Documentation/git-clone-pack.txt
index 87c0e46..b58165a 100644
--- a/Documentation/git-clone-pack.txt
+++ b/Documentation/git-clone-pack.txt
@@ -8,7 +8,7 @@ git-clone-pack - Clones a repository by
SYNOPSIS
--------
-'git-clone-pack' [-q] [--exec=<git-upload-pack>] [<host>:]<directory> [<head>...]
+'git-clone-pack' [-q] [--keep] [--exec=<git-upload-pack>] [<host>:]<directory> [<head>...]
DESCRIPTION
-----------
@@ -23,6 +23,11 @@ OPTIONS
Pass '-q' flag to 'git-unpack-objects'; this makes the
cloning process less verbose.
+--keep::
+ Do not invoke 'git-unpack-objects' on received data, but
+ create a single packfile out of it instead, and store it
+ in the object database.
+
--exec=<git-upload-pack>::
Use this to specify the path to 'git-upload-pack' on the
remote side, if it is not found on your $PATH.
diff --git a/clone-pack.c b/clone-pack.c
index 9567900..2f09df0 100644
--- a/clone-pack.c
+++ b/clone-pack.c
@@ -5,7 +5,8 @@
static int quiet;
static int keep_pack;
-static const char clone_pack_usage[] = "git-clone-pack [-q] [--exec=<git-upload-pack>] [<host>:]<directory> [<heads>]*";
+static const char clone_pack_usage[] =
+"git-clone-pack [-q] [--keep] [--exec=<git-upload-pack>] [<host>:]<directory> [<heads>]*";
static const char *exec = "git-upload-pack";
static void clone_handshake(int fd[2], struct ref *ref)
---
0.99.8.GIT
^ permalink raw reply related
* maybe breakage with latest git-pull and http protocol
From: Randal L. Schwartz @ 2005-10-13 0:53 UTC (permalink / raw)
To: git
I updated git to d06b689a933f6d2130f8afdf1ac0ddb83eeb59ab,
then compiled and installed.
When I went to "git-pull" on my cogito archive (which I had edited
to use HTTP instead of RSYNC), I got into trouble. Unfortunately,
I changed it to rsync to force cogito into a sane state before
I realized that this would be a good bug report. :)
This is perhaps just a heads-up that the recent git-pull might be
broken with respect to http updates.
--
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!
^ permalink raw reply
* [PATCH] Fix packname hash generation.
From: Junio C Hamano @ 2005-10-12 23:57 UTC (permalink / raw)
To: git
In-Reply-To: <20051012135405.CDE55E005E3@center4.mivlgu.local>
This changes the generation of hash packfiles have in their names, from
"hash of object names as fed to us" to "hash of object names in the
resulting pack, in the order they appear in the index file". The new
"git-index-pack" command is taught to output the computed hash value
to its standard output.
With this, we can store downloaded pack in a temporary file without
knowing its final name, run git-index-pack to generate idx for it
while finding out its final name, and then rename the pack and idx to
their final names.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
* Right now, the pack "hash" name only serves the collision
avoidance purposes, but not true identification. The same
set of objects can be fed to pack-objects in different order,
produce the same pack, and still end up with different
names.
This will be used in the next experiment, "git-clone not
exploding the downloaded pack".
index-pack.c | 15 +++++++++++++--
pack-objects.c | 14 ++++++++++----
2 files changed, 23 insertions(+), 6 deletions(-)
applies-to: ea37b42d53264d65f746b3e42577349e8a44d5c4
3b97470e3711d7af3505baddc34428a0d9bd8214
diff --git a/index-pack.c b/index-pack.c
index badbeab..785fe71 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -349,7 +349,7 @@ static int sha1_compare(const void *_a,
return memcmp(a->sha1, b->sha1, 20);
}
-static void write_index_file(const char *index_name)
+static void write_index_file(const char *index_name, unsigned char *sha1)
{
struct sha1file *f;
struct object_entry **sorted_by_sha =
@@ -358,6 +358,7 @@ static void write_index_file(const char
struct object_entry **last = sorted_by_sha + nr_objects;
unsigned int array[256];
int i;
+ SHA_CTX ctx;
for (i = 0; i < nr_objects; ++i)
sorted_by_sha[i] = &objects[i];
@@ -385,6 +386,11 @@ static void write_index_file(const char
}
sha1write(f, array, 256 * sizeof(int));
+ /* recompute the SHA1 hash of sorted object names.
+ * currently pack-objects does not do this, but that
+ * can be fixed.
+ */
+ SHA1_Init(&ctx);
/*
* Write the actual SHA1 entries..
*/
@@ -394,10 +400,12 @@ static void write_index_file(const char
unsigned int offset = htonl(obj->offset);
sha1write(f, &offset, 4);
sha1write(f, obj->sha1, 20);
+ SHA1_Update(&ctx, obj->sha1, 20);
}
sha1write(f, pack_base + pack_size - 20, 20);
sha1close(f, NULL, 1);
free(sorted_by_sha);
+ SHA1_Final(sha1, &ctx);
}
int main(int argc, char **argv)
@@ -405,6 +413,7 @@ int main(int argc, char **argv)
int i;
char *index_name = NULL;
char *index_name_buf = NULL;
+ unsigned char sha1[20];
for (i = 1; i < argc; i++) {
const char *arg = argv[i];
@@ -443,9 +452,11 @@ int main(int argc, char **argv)
deltas = xcalloc(nr_objects, sizeof(struct delta_entry));
parse_pack_objects();
free(deltas);
- write_index_file(index_name);
+ write_index_file(index_name, sha1);
free(objects);
free(index_name_buf);
+ printf("%s\n", sha1_to_hex(sha1));
+
return 0;
}
diff --git a/pack-objects.c b/pack-objects.c
index 3d62278..ef55cab 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -393,6 +393,7 @@ int main(int argc, char **argv)
SHA_CTX ctx;
char line[PATH_MAX + 20];
int window = 10, depth = 10, pack_to_stdout = 0;
+ struct object_entry **list;
int i;
for (i = 1; i < argc; i++) {
@@ -435,7 +436,6 @@ int main(int argc, char **argv)
if (pack_to_stdout != !base_name)
usage(pack_usage);
- SHA1_Init(&ctx);
while (fgets(line, sizeof(line), stdin) != NULL) {
unsigned int hash;
char *p;
@@ -451,10 +451,8 @@ int main(int argc, char **argv)
continue;
hash = hash * 11 + c;
}
- if (add_object_entry(sha1, hash))
- SHA1_Update(&ctx, sha1, 20);
+ add_object_entry(sha1, hash);
}
- SHA1_Final(object_list_sha1, &ctx);
if (non_empty && !nr_objects)
return 0;
get_object_details();
@@ -462,6 +460,14 @@ int main(int argc, char **argv)
fprintf(stderr, "Packing %d objects\n", nr_objects);
sorted_by_sha = create_sorted_list(sha1_sort);
+ SHA1_Init(&ctx);
+ list = sorted_by_sha;
+ for (i = 0; i < nr_objects; i++) {
+ struct object_entry *entry = *list++;
+ SHA1_Update(&ctx, entry->sha1, 20);
+ }
+ SHA1_Final(object_list_sha1, &ctx);
+
sorted_by_type = create_sorted_list(type_size_sort);
if (window && depth)
find_deltas(sorted_by_type, window+1, depth);
---
0.99.8.GIT
^ permalink raw reply related
* [PATCH] Refuse to create funny refs in clone-pack, git-fetch and receive-pack.
From: Junio C Hamano @ 2005-10-12 22:01 UTC (permalink / raw)
To: git
In-Reply-To: <7v4q7mip30.fsf@assigned-by-dhcp.cox.net>
Using git-check-ref-format, make sure we do not create refs with
funny names when cloning from elsewhere (clone-pack), fast forwarding
local heads (git-fetch), or somebody pushes into us (receive-pack).
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
* This is the second installment. Cloning over HTTP uses the
commit walker '-w' ref updates, and should already be covered
by the previous "check_ref_format()" updates.
clone-pack.c | 6 ++++++
git-parse-remote.sh | 6 ++++++
receive-pack.c | 4 ++++
3 files changed, 16 insertions(+), 0 deletions(-)
applies-to: 409fe70483c97167e9b7d396334ec5f96932694d
8da42e4b3363de13ede2f006ad96ee8268e52250
diff --git a/clone-pack.c b/clone-pack.c
index c102ca8..48bee96 100644
--- a/clone-pack.c
+++ b/clone-pack.c
@@ -34,6 +34,12 @@ static void write_one_ref(struct ref *re
int fd;
char *hex;
+ if (!strncmp(ref->name, "refs/", 5) &&
+ check_ref_format(ref->name + 5)) {
+ error("refusing to create funny ref '%s' locally", ref->name);
+ return;
+ }
+
if (safe_create_leading_directories(path))
die("unable to create leading directory for %s", ref->name);
fd = open(path, O_CREAT | O_EXCL | O_WRONLY, 0666);
diff --git a/git-parse-remote.sh b/git-parse-remote.sh
index 5e75e15..aea7b0e 100755
--- a/git-parse-remote.sh
+++ b/git-parse-remote.sh
@@ -94,6 +94,12 @@ canon_refs_list_for_fetch () {
heads/* | tags/* ) local="refs/$local" ;;
*) local="refs/heads/$local" ;;
esac
+
+ if local_ref_name=$(expr "$local" : 'refs/\(.*\)')
+ then
+ git-check-ref-format "$local_ref_name" ||
+ die "* refusing to create funny ref '$local_ref_name' locally"
+ fi
echo "${dot_prefix}${force}${remote}:${local}"
dot_prefix=.
done
diff --git a/receive-pack.c b/receive-pack.c
index 06857eb..8f157bc 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -95,6 +95,10 @@ static int update(const char *name,
char new_hex[60], *old_hex, *lock_name;
int newfd, namelen, written;
+ if (!strncmp(name, "refs/", 5) && check_ref_format(name + 5))
+ return error("refusing to create funny ref '%s' locally",
+ name);
+
namelen = strlen(name);
lock_name = xmalloc(namelen + 10);
memcpy(lock_name, name, namelen);
---
0.99.8.GIT
^ permalink raw reply related
* [PATCH] git-check-ref-format: reject funny ref names.
From: Junio C Hamano @ 2005-10-12 22:01 UTC (permalink / raw)
To: git
In-Reply-To: <7v4q7mip30.fsf@assigned-by-dhcp.cox.net>
Update check_ref_format() function to reject ref names that:
* has a path component that begins with a ".", or
* has ASCII control character, "~", "^", ":" or SP, anywhere, or
* ends with a "/".
Use it in 'git-checkout -b', 'git-branch', and 'git-tag' to make sure
that newly created refs are well-formed.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
* This is the beginning of currently two-patch series. This
one "fixes" the programs that trivially create new refs.
Also check_ref_format() is used by commit walkers when
writing a new ref or updating an existing ref, so this patch
makes them to refuse funny refs being created.
Makefile | 2 +-
check-ref-format.c | 17 ++++++++++++++
git-branch.sh | 63 +++++++++++++++++++++++++++++-----------------------
git-checkout.sh | 2 ++
git-tag.sh | 2 ++
refs.c | 52 +++++++++++++++++++++++++++++++++++--------
6 files changed, 100 insertions(+), 38 deletions(-)
create mode 100644 check-ref-format.c
applies-to: ea37b42d53264d65f746b3e42577349e8a44d5c4
3fa00f52f80e5e51cc8098979ce0883c77caab52
diff --git a/Makefile b/Makefile
index 7c8f647..2860d47 100644
--- a/Makefile
+++ b/Makefile
@@ -120,7 +120,7 @@ PROGRAMS = \
git-ssh-upload$X git-tar-tree$X git-unpack-file$X \
git-unpack-objects$X git-update-index$X git-update-server-info$X \
git-upload-pack$X git-verify-pack$X git-write-tree$X \
- git-update-ref$X git-symbolic-ref$X \
+ git-update-ref$X git-symbolic-ref$X git-check-ref-format$X \
$(SIMPLE_PROGRAMS)
# Backward compatibility -- to be removed after 1.0
diff --git a/check-ref-format.c b/check-ref-format.c
new file mode 100644
index 0000000..a0adb3d
--- /dev/null
+++ b/check-ref-format.c
@@ -0,0 +1,17 @@
+/*
+ * GIT - The information manager from hell
+ */
+
+#include "cache.h"
+#include "refs.h"
+
+#include <stdio.h>
+
+int main(int ac, char **av)
+{
+ if (ac != 2)
+ usage("git-check-ref-format refname");
+ if (check_ref_format(av[1]))
+ exit(1);
+ return 0;
+}
diff --git a/git-branch.sh b/git-branch.sh
index 074229c..e2db906 100755
--- a/git-branch.sh
+++ b/git-branch.sh
@@ -13,38 +13,42 @@ If two arguments, create a new branch <b
}
delete_branch () {
- option="$1" branch_name="$2"
+ option="$1"
+ shift
headref=$(GIT_DIR="$GIT_DIR" git-symbolic-ref HEAD |
sed -e 's|^refs/heads/||')
- case ",$headref," in
- ",$branch_name,")
- die "Cannot delete the branch you are on." ;;
- ,,)
- die "What branch are you on anyway?" ;;
- esac
- branch=$(cat "$GIT_DIR/refs/heads/$branch_name") &&
- branch=$(git-rev-parse --verify "$branch^0") ||
- die "Seriously, what branch are you talking about?"
- case "$option" in
- -D)
- ;;
- *)
- mbs=$(git-merge-base -a "$branch" HEAD | tr '\012' ' ')
- case " $mbs " in
- *' '$branch' '*)
- # the merge base of branch and HEAD contains branch --
- # which means that the HEAD contains everything in the HEAD.
+ for branch_name
+ do
+ case ",$headref," in
+ ",$branch_name,")
+ die "Cannot delete the branch you are on." ;;
+ ,,)
+ die "What branch are you on anyway?" ;;
+ esac
+ branch=$(cat "$GIT_DIR/refs/heads/$branch_name") &&
+ branch=$(git-rev-parse --verify "$branch^0") ||
+ die "Seriously, what branch are you talking about?"
+ case "$option" in
+ -D)
;;
*)
- echo >&2 "The branch '$branch_name' is not a strict subset of your current HEAD.
-If you are sure you want to delete it, run 'git branch -D $branch_name'."
- exit 1
+ mbs=$(git-merge-base -a "$branch" HEAD | tr '\012' ' ')
+ case " $mbs " in
+ *' '$branch' '*)
+ # the merge base of branch and HEAD contains branch --
+ # which means that the HEAD contains everything in the HEAD.
+ ;;
+ *)
+ echo >&2 "The branch '$branch_name' is not a strict subset of your current HEAD.
+ If you are sure you want to delete it, run 'git branch -D $branch_name'."
+ exit 1
+ ;;
+ esac
;;
esac
- ;;
- esac
- rm -f "$GIT_DIR/refs/heads/$branch_name"
- echo "Deleted branch $branch_name."
+ rm -f "$GIT_DIR/refs/heads/$branch_name"
+ echo "Deleted branch $branch_name."
+ done
exit 0
}
@@ -52,7 +56,7 @@ while case "$#,$1" in 0,*) break ;; *,-*
do
case "$1" in
-d | -D)
- delete_branch "$1" "$2"
+ delete_branch "$@"
exit
;;
--)
@@ -93,6 +97,9 @@ branchname="$1"
rev=$(git-rev-parse --verify "$head") || exit
-[ -e "$GIT_DIR/refs/heads/$branchname" ] && die "$branchname already exists"
+[ -e "$GIT_DIR/refs/heads/$branchname" ] &&
+ die "$branchname already exists."
+git-check-ref-format "heads/$branchname" ||
+ die "we do not like '$branchname' as a branch name."
echo $rev > "$GIT_DIR/refs/heads/$branchname"
diff --git a/git-checkout.sh b/git-checkout.sh
index c382590..2c053a3 100755
--- a/git-checkout.sh
+++ b/git-checkout.sh
@@ -17,6 +17,8 @@ while [ "$#" != "0" ]; do
die "git checkout: -b needs a branch name"
[ -e "$GIT_DIR/refs/heads/$newbranch" ] &&
die "git checkout: branch $newbranch already exists"
+ git-check-ref-format "heads/$newbranch" ||
+ die "we do not like '$newbranch' as a branch name."
;;
"-f")
force=1
diff --git a/git-tag.sh b/git-tag.sh
index 25c1a0e..faa7667 100755
--- a/git-tag.sh
+++ b/git-tag.sh
@@ -53,6 +53,8 @@ if [ -e "$GIT_DIR/refs/tags/$name" -a -z
die "tag '$name' already exists"
fi
shift
+git-check-ref-format "tags/$name" ||
+ die "we do not like '$name' as a tag name."
object=$(git-rev-parse --verify --default HEAD "$@") || exit 1
type=$(git-cat-file -t $object) || exit 1
diff --git a/refs.c b/refs.c
index 5a8cbd4..2d2144c 100644
--- a/refs.c
+++ b/refs.c
@@ -335,17 +335,51 @@ int write_ref_sha1(const char *ref, int
return retval;
}
+/*
+ * Make sure "ref" is something reasonable to have under ".git/refs/";
+ * We do not like it if:
+ *
+ * - any path component of it begins with ".", or
+ * - it has ASCII control character, "~", "^", ":" or SP, anywhere, or
+ * - it ends with a "/".
+ */
+
+static inline int bad_ref_char(int ch)
+{
+ return (((unsigned) ch) <= ' ' ||
+ ch == '~' || ch == '^' || ch == ':');
+}
+
int check_ref_format(const char *ref)
{
- char *middle;
- if (ref[0] == '.' || ref[0] == '/')
- return -1;
- middle = strchr(ref, '/');
- if (!middle || !middle[1])
- return -1;
- if (strchr(middle + 1, '/'))
- return -1;
- return 0;
+ int ch, level;
+ const char *cp = ref;
+
+ level = 0;
+ while (1) {
+ while ((ch = *cp++) == '/')
+ ; /* tolerate duplicated slashes */
+ if (!ch)
+ return -1; /* should not end with slashes */
+
+ /* we are at the beginning of the path component */
+ if (ch == '.' || bad_ref_char(ch))
+ return -1;
+
+ /* scan the rest of the path component */
+ while ((ch = *cp++) != 0) {
+ if (bad_ref_char(ch))
+ return -1;
+ if (ch == '/')
+ break;
+ }
+ level++;
+ if (!ch) {
+ if (level < 2)
+ return -1; /* at least of form "heads/blah" */
+ return 0;
+ }
+ }
}
int write_ref_sha1_unlocked(const char *ref, const unsigned char *sha1)
---
0.99.8.GIT
^ permalink raw reply related
* diff_tree_stdin
From: Morten Welinder @ 2005-10-12 1:46 UTC (permalink / raw)
To: GIT Mailing List
It looks like diff_tree_stdin can overrun the this_header buffer. Since the
line length is already calculated, a check would be cheap.
Morten
^ permalink raw reply
* Re: [PATCH] Try URI quoting for embedded TAB and LF in pathnames
From: Junio C Hamano @ 2005-10-12 21:33 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0510121355280.15297@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> This is really what my argument boils down to: character set encoding
> should _not_ EVER affect the _transfer_ of the data. It doesn't matter if
> something is latin1 or utf-8, the only thing that matters is the byte
> sequence. Only when you _display_ it should you try to figure out what the
> byte sequence possibly means.
>
> So I repeat:
> - escape as little as possible
> - make the _viewer_ decide how to view it.
I think the same argument can be made about patch application,
although strictly speaking it is not "viewing". Let the patch
program decide (or the user to tell her decision to the patch
program) what the unescaped byte sequence in the patch that
represents the path being affected is encoded in, and do
something sensible while taking into account that the pathname
encoding on the working tree may be different from what is
recorded in the patch.
For example, one of my partitions is ntfs mounted with
nls=euc-jp, and I expect the tool to help me apply patches to a
Japanese-named file when the patch is from a system with UTF-8
encoded filenames.
^ permalink raw reply
* Re: [PATCH] Try URI quoting for embedded TAB and LF in pathnames
From: Linus Torvalds @ 2005-10-12 21:24 UTC (permalink / raw)
To: Paul Eggert
Cc: Junio C Hamano, Robert Fitzsimons, Alex Riesen, git, Kai Ruemmler
In-Reply-To: <87vf02qy79.fsf@penguin.cs.ucla.edu>
On Wed, 12 Oct 2005, Paul Eggert wrote:
>
> Worse, when I used Emacs to copy your text into another file -- the
> sort of thing that is likely to be done with an emailed patch -- the
> file contained the UTF-8 encoding of the gibberish, rather than the
> original bytes of your message.
Btw, this is an example of where locale-based character translations just
fundamentally suck.
cut-and-paste quote naturally tries to translate between the source
and destination locales, but it fundamentally cannot work. The only thing
that ever works is bit-for-bit copying.
Any program that tries to do locale conversion is always going to be a bug
waiting to happen.
If GNU emacs does locale translations rather than just do a binary
transfer of the data, then that's a sign that GNU emavs is being really
stupid. If the data was UTF-8 to begin with, then a binary copy is also
going to be UTF-8. And if it wasn't UTF-8, then a binary copy is the only
thing that is sensible.
And this is the thing that makes UTF-8 so wonderful: exactly the fact that
it makes bit-for-bit copying an acceptable policy again, and locales
become a non-issue. In a truly UTF-8 world, you should _never_ convert
anything at all (and that includes mis-formed UTF-8).
Any non-binary file saving or transfer approach where characters have
"meaning" is always mistake. It's why DOS/Windows "binary" vs "text" files
was wrong. It's why font-encoding locales are wrong (Mixed text with two
types? Yet another metadata quoting scheme? No thank you! It's also why
UCS-16 and UCS-32 were total disasters: they had "context" in their
encoding).
Say "yes" to binary transfer. Because text transfers are broken.
Linus
^ permalink raw reply
* Re: [PATCH] Try URI quoting for embedded TAB and LF in pathnames
From: Johannes Schindelin @ 2005-10-12 21:15 UTC (permalink / raw)
To: Linus Torvalds
Cc: Paul Eggert, Junio C Hamano, Robert Fitzsimons, Alex Riesen, git,
Kai Ruemmler
In-Reply-To: <Pine.LNX.4.64.0510121355280.15297@g5.osdl.org>
Hi,
On Wed, 12 Oct 2005, Linus Torvalds wrote:
> Yes, if people use "cat" to view patches, it can be dangerous. But that's
> _their_ problem.
No, that is the cat's problem. Sorry, couldn't resist.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Try URI quoting for embedded TAB and LF in pathnames
From: H. Peter Anvin @ 2005-10-12 21:09 UTC (permalink / raw)
To: Linus Torvalds
Cc: Paul Eggert, Junio C Hamano, Robert Fitzsimons, Alex Riesen, git,
Kai Ruemmler
In-Reply-To: <Pine.LNX.4.64.0510121355280.15297@g5.osdl.org>
Linus Torvalds wrote:
>
> Now, I believe patches can actually be that way - it's not at all
> impossible to have a diff where the _filename_ is utf-8, but the content
> of the patch itself is some byte-encoding like latin1. Or the other way
> around.
>
Or both. Trivial example: a patch to change names in comments from ISO
8859-1 to UTF-8.
-hpa
^ permalink raw reply
* Re: [PATCH] Try URI quoting for embedded TAB and LF in pathnames
From: Linus Torvalds @ 2005-10-12 21:05 UTC (permalink / raw)
To: Paul Eggert
Cc: Junio C Hamano, Robert Fitzsimons, Alex Riesen, git, Kai Ruemmler
In-Reply-To: <87vf02qy79.fsf@penguin.cs.ucla.edu>
On Wed, 12 Oct 2005, Paul Eggert wrote:
>
> Your email message suggests that we need to be cautious here.
> That message contained UTF-8 text but its header said "Content-Type:
> TEXT/PLAIN; charset=ISO-8859-1".
Well, my email message was wrong and evil, because it _mixed_ two
different encodings in the same text. No sane client could have shown them
both at the same time - but especially with a stupid client, you could
have changed your terminal to show either one or the other by switching
from utf-8 to latin1 encoding and doing a refresh.
In other words, my email really was a nasty case of not one or the other,
but both.
Now, I believe patches can actually be that way - it's not at all
impossible to have a diff where the _filename_ is utf-8, but the content
of the patch itself is some byte-encoding like latin1. Or the other way
around.
> If we're still having problems like this in 2005 then I guess we need
> to deal with them. This suggests we should be escaping every
> non-ASCII byte, at least for patches designed to be emailed robustly.
I find that email is very robust - it's basically 8-bit clean. No
character encoding, no crap. Just a byte stream. It really _is_ the most
reliable format.
Now, a lot of email clients are really weak in _showing_ it, and as
mentioned, the email that mixed both is fundamentally not something you
really even _can_ show sanely. But who cares? What matters is not what it
looks like, but what it _saves_ as. If you save the email message, it
should come out as the same reliable 8-bit byte stream, or your client is
actively corrupting messages rather than just showing them.
This is really what my argument boils down to: character set encoding
should _not_ EVER affect the _transfer_ of the data. It doesn't matter if
something is latin1 or utf-8, the only thing that matters is the byte
sequence. Only when you _display_ it should you try to figure out what the
byte sequence possibly means.
So I repeat:
- escape as little as possible
- make the _viewer_ decide how to view it.
Yes, if people use "cat" to view patches, it can be dangerous. But that's
_their_ problem.
Linus
^ permalink raw reply
* Re: [PATCH] Try URI quoting for embedded TAB and LF in pathnames
From: Junio C Hamano @ 2005-10-12 21:02 UTC (permalink / raw)
To: Paul Eggert
Cc: Linus Torvalds, Robert Fitzsimons, Alex Riesen, git, Kai Ruemmler
In-Reply-To: <87vf02qy79.fsf@penguin.cs.ucla.edu>
Paul Eggert <eggert@CS.UCLA.EDU> writes:
> Linus Torvalds <torvalds@osdl.org> writes:
>
>> I don't know if you realize it, but it's only within the last couple of
>> years that the old 7-bit "finnish ASCII" went away.
>
> Aach! Those Finns! Always on the trailing edge of technology!
Nah, Japanese are much worse. We are so used to see Yen signs
at the end of multi-line CPP macro definitions (backslashes are
taken over by it) and I do not foresee it going away anytime
soon. I think windows people believe Yen signs are path
component separators ;-).
^ permalink raw reply
* Re: [PATCH] Try URI quoting for embedded TAB and LF in pathnames
From: H. Peter Anvin @ 2005-10-12 20:21 UTC (permalink / raw)
To: Linus Torvalds
Cc: Daniel Barkalow, Paul Eggert, Junio C Hamano, Robert Fitzsimons,
Alex Riesen, git, Kai Ruemmler
In-Reply-To: <Pine.LNX.4.64.0510121220230.15297@g5.osdl.org>
Linus Torvalds wrote:
>
> Nope. The traditional vt100 escape sequence is "ESC" followed by a
> character to indicate the type of sequence (the most common one is '[').
> That's all 7-bit and fine.
>
> HOWEVER, they made the 8-bit extension be such that any of these vt100
> begin sequences where the second character is in the appropriate range can
> be instead shortened by one character, by instead using a single 8-bit
> character of "0x80+(char-0x40)". Ie the traditional "ESC + '['" (\x1b\x5b)
> can also be written as a single '\x9b' character, aka CSI.
>
> In other words, 0x80-0x9f are _all_ just vt100 shorthand for ESC+'@'
> through ESC+'_'.
>
> (I guess it's not strictly "vt100" any more - it's the extended vt220
> format).
>
Actually, it's even trickier than that.
CSI is character 0x1b of control code set C1; there are two "windows"
for control codes -- CL (0x00-0x1f) and CR (0x80-0x9f). Normally CL is
mapped to C0 and CR is mapped to CL, but ESC will temporarily map C1
into CL.
VT1xx didn't support this since they didn't support 8-bit anything.
Anyway, a *lot* of character sets -- not just UTF-8 -- use the CR range
of bytes for printables.
-hpa
^ permalink raw reply
* Re: [PATCH] Try URI quoting for embedded TAB and LF in pathnames
From: Linus Torvalds @ 2005-10-12 19:52 UTC (permalink / raw)
To: Daniel Barkalow
Cc: Paul Eggert, Junio C Hamano, Robert Fitzsimons, Alex Riesen, git,
Kai Ruemmler
In-Reply-To: <Pine.LNX.4.63.0510121452030.23242@iabervon.org>
On Wed, 12 Oct 2005, Daniel Barkalow wrote:
>
> I think it's actually sufficient to escape 0x00-0x1f and 0x7f; those
> ranges are both easy
They are indeed easy.
> and, as far as I can tell, include all of the control
> characters that do annoying things.
Nope. The traditional vt100 escape sequence is "ESC" followed by a
character to indicate the type of sequence (the most common one is '[').
That's all 7-bit and fine.
HOWEVER, they made the 8-bit extension be such that any of these vt100
begin sequences where the second character is in the appropriate range can
be instead shortened by one character, by instead using a single 8-bit
character of "0x80+(char-0x40)". Ie the traditional "ESC + '['" (\x1b\x5b)
can also be written as a single '\x9b' character, aka CSI.
In other words, 0x80-0x9f are _all_ just vt100 shorthand for ESC+'@'
through ESC+'_'.
(I guess it's not strictly "vt100" any more - it's the extended vt220
format).
> I think escape, backspace, delete, and
> bell are the only ones we'd rather the terminal not get; beyond that,
> patches with screwy filenames look screwy, but don't screw up anything
> outside of the filename.
Try this on a (non-UTF-8) xterm:
echo -en '\x9b5B---\x9b1A---\x9b4A\r'
and it should do:
- move cursor 5 lines down
- print "---"
- move cursor 1 line up
- print "---"
- move cursor 4 lines up
- return carriage to beginning.
In other words, your screen should end up looking something like this:
[torvalds@g5 ~]$ echo -en '\x9b5B---\x9b1A---\x9b4A\r'
[torvalds@g5 ~]$
---
---
where that "staircase" of two "---" things was done with cursor movements.
And that's a _benign_ sequence. You can do all kinds of funky stuff that
really screws up the user experience. Including have the thing echo keys
to you that you didn't type:
echo -en '\x9b5n'
or lock the keyboard (I don't think any of the terminal emulators
implement the latter, or some of the other stranger sequences - things to
do double-wide characters etc).
Linus
PS. You can do all the same in UTF-8 one, but then you'll have to add a
\xc2 before the \x9b:
echo -en '\xc2\x9b5B---\xc2\x9b1A---\xc2\x9b4A\r'
etc..
^ permalink raw reply
* Re: [PATCH] Try URI quoting for embedded TAB and LF in pathnames
From: Daniel Barkalow @ 2005-10-12 19:07 UTC (permalink / raw)
To: Linus Torvalds
Cc: Paul Eggert, Junio C Hamano, Robert Fitzsimons, Alex Riesen, git,
Kai Ruemmler
In-Reply-To: <Pine.LNX.4.64.0510120749230.14597@g5.osdl.org>
On Wed, 12 Oct 2005, Linus Torvalds wrote:
> So if you want to support any other locale than UTF-8, you need to escape
> them. Assuming you want to escape control characters at all, of course (I
> still think it's perfectly fine to just let the raw mess through and
> depend on escaping at higher levels)
I think it's actually sufficient to escape 0x00-0x1f and 0x7f; those
ranges are both easy and, as far as I can tell, include all of the control
characters that do annoying things. I think escape, backspace, delete, and
bell are the only ones we'd rather the terminal not get; beyond that,
patches with screwy filenames look screwy, but don't screw up anything
outside of the filename.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH] git-fetch --tags: deal with tags with spaces in them.
From: Junio C Hamano @ 2005-10-12 18:57 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: git, Martin Langhoff (CatalystIT)
In-Reply-To: <434D3000.5020601@zytor.com>
"H. Peter Anvin" <hpa@zytor.com> writes:
> H. Peter Anvin wrote:
>> We can disallow whitespace, and we *have* to disallow at least
>> newline due to the file format; I believe we should disallow all
>> control characters (0-31, 127-159.)
>
> Actually, disallowing anything 128 and above means knowing the encoding
> system. If we enforce UTF-8, we should presumably disallow at the very
> least U+FFFE and U+FFFF too.
Hmph. I think enforcing (or rather supporting preferentially)
UTF-8 in log messages was alright, but enforcing UTF-8 tagnames
imply UTF-8 host pathnames because we do not currently convert
when we fetch refs from remote and store locally.
* git-clone-pack, git-fetch-pack and git-peek-remote run
git-upload-pack on the other end. Currently upload-pack
sends a list of refs read from the remote filesystem without
conversion, and:
(1) clone-pack uses the names without conversion to replicate
refs on the local filesystem.
(2) fetch-pack sends the names given on the command line,
and/or read from the local filesystem, to upload-pack
without conversion.
(3) fetch-pack and peek-remote outputs the names obtained
from the remote without conversion to stdout.
* over http, the encoding of the refnames client sees is what
is stored in project.git/info/refs on the remote. Currently,
update-server-info reads from the filesystem and writes this
file out without conversion. While walking the commits,
names are not used, so there is no refname encoding issues.
What we should do at this point is to declare that exchanging
refnames between systems is to happen after converting them to
UTF-8. And version 1.0 just assumes pathnames are UTF-8.
If people on systems with non UTF-8 pathnames cared enough, the
tools can be made aware of local pathname encodings, and taught
how to convert what for_each_ref() read from the filesystem, the
refspecs given from the command line, etc. to UTF-8. But that
can come later.
^ permalink raw reply
* Re: [PATCH] git-fetch --tags: deal with tags with spaces in them.
From: Junio C Hamano @ 2005-10-12 18:10 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: git, Martin Langhoff (CatalystIT)
In-Reply-To: <434D2D8F.2020407@zytor.com>
"H. Peter Anvin" <hpa@zytor.com> writes:
> We can disallow whitespace, and we *have* to disallow at least newline
> due to the file format; I believe we should disallow all control
> characters (0-31, 127-159.)
I agree.
Currently SP, TAB, and LF are all we care about, and nothing
else; NUL does not even count. But if we are codifying a list
of disallowed byte values, excluding all control characters is
probably a sane thing to do, without introducing arbitrary and
unnecessary limitation too much.
^ permalink raw reply
* Re: [PATCH] gitk: Add "Refs" menu
From: H. Peter Anvin @ 2005-10-12 17:47 UTC (permalink / raw)
To: Marco Costalba; +Cc: Pavel Roskin, git
In-Reply-To: <20051012115559.6546.qmail@web26305.mail.ukl.yahoo.com>
Marco Costalba wrote:
>
>
>>And making gitk cooperate with stgit would be a killer application not
>>just for gitk and stgit, but for git itself (i.e. it could be the reason
>>why git is chosen for development over e.g. Mercurial for new projects).
>>
>
>
> Not to advertise, but qgit (http://digilander.libero.it/mcostalba/) already offers
> stgit integration, among other things.
>
> I plan to release a new version implementing various suggestion from the list this week, there are
> also important stgit fixes and upgrades.
>
> To have a look at new features check the git arcihve: cg-clone
> http://digilander.libero.it/mcostalba/qgit.git
>
I looked at qgit at one time, and I find its UI to be much less pleasant
to look at that gitk, which is unfortunate, because it's much faster.
Both the graph and the color scheme are much less pleasant, which
distracts from the information conveyed.
-hpa
^ permalink raw reply
* Re: [PATCH] gitk: Add "Refs" menu
From: Pavel Roskin @ 2005-10-12 16:03 UTC (permalink / raw)
To: Marco Costalba; +Cc: git
In-Reply-To: <20051012115559.6546.qmail@web26305.mail.ukl.yahoo.com>
On Wed, 2005-10-12 at 04:55 -0700, Marco Costalba wrote:
> > And making gitk cooperate with stgit would be a killer application not
> > just for gitk and stgit, but for git itself (i.e. it could be the reason
> > why git is chosen for development over e.g. Mercurial for new projects).
> >
>
> Not to advertise, but qgit (http://digilander.libero.it/mcostalba/) already offers
> stgit integration, among other things.
Very nice.
But it doesn't show correctly in the qt theme I was using (which must be
the default theme in Fedora Core 4 because I never changed it):
http://red-bean.com/proski/qgit.png
The blame for the ugly default gray background should be on Qt or on
Fedora, but it seems to me that qgit is hardcoding white and light gray
(see ODD_LINE_COL and EVEN_LINE_COL in src/mainimpl.cpp).
The style selector in kcontrol has "Standard Background" and "Alternate
Background for Lists". I don't know if it has an equivalent in qt.
Maybe you could use the standard background and a slightly darkened
version.
Also, please use months names on the homepage. I thought qgit 0.94 was
released on October 9th.
--
Regards,
Pavel Roskin
^ permalink raw reply
* Re: [PATCH] git-fetch --tags: deal with tags with spaces in them.
From: H. Peter Anvin @ 2005-10-12 15:47 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: Junio C Hamano, git, Martin Langhoff (CatalystIT)
In-Reply-To: <434D2D8F.2020407@zytor.com>
H. Peter Anvin wrote:
>
> We can disallow whitespace, and we *have* to disallow at least newline
> due to the file format; I believe we should disallow all control
> characters (0-31, 127-159.)
>
Actually, disallowing anything 128 and above means knowing the encoding
system. If we enforce UTF-8, we should presumably disallow at the very
least U+FFFE and U+FFFF too.
C99 contains a list of Unicode characters allowed in C identifiers. I
believe we should allow those characters (plus at a minimum -+.#:@) as
well, but I'd much rather we didn't make those kinds of decisions for
the user. If we have *specific* characters we can't tolerate we should
rule them out (like control characters), but even that occationally
leads to major frustration on the part of the user: "why can't I use '.'
in tag names in CVS"?
-hpa
^ permalink raw reply
* Re: [PATCH] git-fetch --tags: deal with tags with spaces in them.
From: H. Peter Anvin @ 2005-10-12 15:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Martin Langhoff (CatalystIT)
In-Reply-To: <7vk6gjl2uu.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
>
> Here is mine.
>
> I do not personally think it is too much of a restriction if we
> said we only allow tags using letters from [-a-zA-Z0-9.] (yes I
> am trying to be controversial by not allowing even latin-1
> names). Even without going that far, if we just said we do not
> allow shell metacharacters in tagnames (i.e. assuming UTF-8
> encoded pathnames, non-ascii part of unicode character space is
> allowed), I suspect things will get much simpler to handle.
>
> The troublesome tags Martin's repository had were autocreated
> with cvsimport. That is something we could easily fix (I think
> we already do certain tagname munging).
>
I don't know about this. Saying we only support ASCII tagnames can be
quote brutal for non-English-language projects. Even with the set
above, we'd at the very least need underscore, and I know of at least
one project which require # in tagnames. In short, I don't think we can
make that decision for people.
We can disallow whitespace, and we *have* to disallow at least newline
due to the file format; I believe we should disallow all control
characters (0-31, 127-159.)
-hpa
^ permalink raw reply
* Re: [PATCH] cogito: use locale date_fmt in obtaining default date format
From: Junichi Uekawa @ 2005-10-12 15:26 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <878xwyomth.dancerj%dancer@netfort.gr.jp>
Hi,
> if [ "$has_gnudate" ]; then
> LANG=C $has_gnudate -ud "1970-01-01 UTC + $secs sec" "$format"
> else
Hmm... I just noticed that you're setting LANG=C here;
which is not enough since LC_ALL will override the value.
I'm not quite sure of the original intention;
I've checked the users
cg-log:
For local display; it should be okay to show the date in
localized format.
cg-mkpatch:
It might be intrusive to have a localized date in a patch;
so having a localized output of showdate is mostly undesirable,
but it will depend on the project you work on.
(The user can always unset locale)
regards,
junichi
^ permalink raw reply
* Re: [PATCH] Add '--create-index' to git-unpack-objects
From: Linus Torvalds @ 2005-10-12 15:20 UTC (permalink / raw)
To: Sergey Vlasov; +Cc: Johannes Schindelin, git, junkio
In-Reply-To: <20051012145548.GA2539@master.mivlgu.local>
On Wed, 12 Oct 2005, Sergey Vlasov wrote:
>
> Hmm, pack-objects.c:write_one() does exactly the opposite - it writes
> the base object _after_ writing out the delta (but it does not ensure
> that ordering completely, so references to base objects can be
> pointing in both directions). Why?
pack-objects.c is actually going to some trouble to make sure that the
resulting pack is "optimal" in layout for the most recent case.
Not that I have actually verified optimality, but it was _meant_ to be
that way. And my limited tests seemed to agree.
So it writes out all objects in "recency order", which is the order it
gets them from git-rev-list: it's the same order as the objects are
discovered when we traverse the history in time (except all commits come
first, since most operations will traverse the commit history more than
they will traverse the rest of the object links).
So the objects that are reachable in the most recent tree are all supposed
to be at the beginning of the pack-file, just after the commits.
Now, think about what happens if such an object is a delta against
something else...
In other words, if the most recent tree contains a delta against a much
older object, we want not only the _delta_ to be early in the pack-file,
we want the object that it is a delta _against_ to be there too (just
_after_ the delta, to be exact: we obviously read the delta first, so it
should come first in the pack).
The point being, that if you unpack the latest tree (ie "git checkout" or
any of the normal "git diff" behaviour), the pack-file will basically be
walked in a dense manner, and linearly starting roughly from the
beginning. Which is the optimal IO pattern. Dense and ascending reads.
Now, if the object is reachable through some recent branch, but the delta
is not, then that is not true. In that case, you want to write the recent
base object early in the pack-file, but you do _not_ want to write the
delta together with it, because that would be the wrong thing for the
"recent head" case: it would add stuff to the beginning of the pack-file
that isn't needed for recent objects.
So that's why it's an assymmetric thing. The preferred ordering of time
breaks the symmetry.
Linus
^ 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