* [PATCH] Port git-tag.sh to C.
@ 2007-06-08 21:38 Kristian Høgsberg
2007-06-08 21:51 ` Johannes Schindelin
0 siblings, 1 reply; 21+ messages in thread
From: Kristian Høgsberg @ 2007-06-08 21:38 UTC (permalink / raw)
To: git; +Cc: Kristian Høgsberg
A more or less straight-forward port of git-tag.sh to C.
Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
There's two behavioral changes here: git tag with no arguments behaves like
git branch with no arguments; it lists all tags. The other change is
an empty line in the tag message template to make it look more like the
commit message template.
cheers,
Kristian
Makefile | 3 +-
builtin-tag.c | 364 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
builtin.h | 1 +
git-tag.sh | 183 -----------------------------
git.c | 1 +
5 files changed, 368 insertions(+), 184 deletions(-)
create mode 100644 builtin-tag.c
delete mode 100755 git-tag.sh
diff --git a/Makefile b/Makefile
index 0f75955..bb1bed1 100644
--- a/Makefile
+++ b/Makefile
@@ -205,7 +205,7 @@ SCRIPT_SH = \
git-pull.sh git-rebase.sh \
git-repack.sh git-request-pull.sh git-reset.sh \
git-sh-setup.sh \
- git-tag.sh git-verify-tag.sh \
+ git-verify-tag.sh \
git-am.sh \
git-merge.sh git-merge-stupid.sh git-merge-octopus.sh \
git-merge-resolve.sh git-merge-ours.sh \
@@ -372,6 +372,7 @@ BUILTIN_OBJS = \
builtin-show-branch.o \
builtin-stripspace.o \
builtin-symbolic-ref.o \
+ builtin-tag.o \
builtin-tar-tree.o \
builtin-unpack-objects.o \
builtin-update-index.o \
diff --git a/builtin-tag.c b/builtin-tag.c
new file mode 100644
index 0000000..06eafc0
--- /dev/null
+++ b/builtin-tag.c
@@ -0,0 +1,364 @@
+/*
+ * Builtin "git tag"
+ *
+ * Copyright (c) 2007 Kristian Høgsberg <krh@redhat.com>
+ * Based on git-tag.sh and mktag.c by Linus Torvalds.
+ */
+
+#include "cache.h"
+#include "refs.h"
+#include "commit.h"
+#include "builtin.h"
+#include "tag.h"
+#include "run-command.h"
+
+static const char builtin_tag_usage[] =
+ "git-tag [-n [<num>]] -l [<pattern>] | [-a | -s | -u <key-id>] [-f | -d | -v] [-m <msg>] <tagname> [<head>]";
+
+static char signingkey[1000];
+
+static int launch_editor(const char *path, const char *template,
+ char *buffer, size_t size)
+{
+ struct child_process child;
+ const char *editor;
+ const char *args[3];
+ char *eol;
+ int len, fd, blank_lines, i, j;
+
+ fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0644);
+ if (fd < 0)
+ die("could not create file %s.", path);
+
+ len = strlen(template);
+ write_or_die(fd, template, len);
+ close(fd);
+
+ editor = getenv("VISUAL");
+ if (!editor)
+ editor = getenv("EDITOR");
+ if (!editor)
+ editor = "vi";
+
+ memset(&child, 0, sizeof(child));
+ child.argv = args;
+ args[0] = editor;
+ args[1] = path;
+ args[2] = NULL;
+
+ if (run_command(&child))
+ die("could not launch editor %s.", editor);
+
+ fd = open(path, O_RDONLY, 0644);
+ if (fd == -1)
+ die("could not read %s.", path);
+ len = read_in_full(fd, buffer, size);
+ if (len < 0)
+ die("failed to read '%s', %m", path);
+ close(fd);
+
+ blank_lines = 1;
+ for (i = 0, j = 0; i < len; i++) {
+ if (blank_lines > 0 && buffer[i] == '#') {
+ eol = strchr(buffer + i, '\n');
+ if (!eol)
+ break;
+
+ i = eol - buffer;
+ continue;
+ }
+
+ if (buffer[i] == '\n') {
+ blank_lines++;
+ if (blank_lines > 1)
+ continue;
+ } else {
+ if (blank_lines > 2)
+ buffer[j++] = '\n';
+ blank_lines = 0;
+ }
+
+ buffer[j++] = buffer[i];
+ }
+
+ if (buffer[j - 1] != '\n')
+ buffer[j++] = '\n';
+
+ unlink(path);
+
+ return j;
+}
+
+static int show_reference(const char *refname, const unsigned char *sha1,
+ int flag, void *cb_data)
+{
+ const char *pattern = cb_data;
+
+ if (pattern == NULL || !fnmatch(pattern, refname, 0))
+ printf("%s\n", refname);
+
+ return 0;
+}
+
+static int list_tags(const char *pattern)
+{
+ for_each_tag_ref(show_reference, (void *) pattern);
+
+ return 0;
+}
+
+
+static int delete_tags(const char **argv)
+{
+ const char **p;
+ char ref[PATH_MAX];
+ int had_error = 0;
+ unsigned char sha1[20];
+
+ for (p = argv; *p; p++) {
+ if (snprintf(ref, sizeof ref, "refs/tags/%s", *p) > sizeof ref)
+ die("tag name '%s' too long.", *p);
+ if (!resolve_ref(ref, sha1, 1, NULL)) {
+ fprintf(stderr, "tag '%s' not found.\n", *p);
+ had_error = 1;
+ continue;
+ }
+
+ if (!delete_ref(ref, sha1))
+ printf("Deleted tag '%s'\n", *p);
+ }
+
+ return had_error;
+}
+
+static int verify_tags(const char **argv)
+{
+ const char **p;
+ char ref[PATH_MAX];
+ int had_error = 0;
+ unsigned char sha1[20];
+
+ for (p = argv; *p; p++) {
+ if (snprintf(ref, sizeof ref, "refs/tags/%s", *p) > sizeof ref)
+ die("tag name '%s' too long.", *p);
+
+ if (!resolve_ref(ref, sha1, 1, NULL)) {
+ fprintf(stderr, "tag '%s' not found.\n", *p);
+ had_error = 1;
+ continue;
+ }
+
+ printf("FIXME: verify tag '%s'\n", *p);
+ }
+
+ return had_error;
+}
+
+static int do_sign(char *buffer, size_t size, size_t max)
+{
+ struct child_process gpg;
+ const char *args[5];
+ char *bracket;
+ int len;
+
+ if (signingkey[0] == '\0') {
+ strlcpy(signingkey, git_committer_info(1), sizeof signingkey);
+ bracket = strchr(signingkey, '>');
+ if (bracket)
+ bracket[1] = '\0';
+ }
+
+ memset(&gpg, 0, sizeof(gpg));
+ gpg.argv = args;
+ gpg.in = -1;
+ gpg.out = -1;
+ args[0] = "gpg";
+ args[1] = "-bsa";
+ args[2] = "-u";
+ args[3] = signingkey;
+ args[4] = NULL;
+
+ if (start_command(&gpg))
+ die("could not run gpg.");
+
+ write_or_die(gpg.in, buffer, size);
+ close(gpg.in);
+ gpg.close_in = 0;
+ len = read_in_full(gpg.out, buffer + size, max - size);
+
+ finish_command(&gpg);
+
+ return size + len;
+}
+
+static const char tag_template[] =
+ "\n"
+ "#\n"
+ "# Write a tag message\n"
+ "#\n";
+
+int git_tag_config(const char *var, const char *value)
+{
+ if (!strcmp(var, "user.signingkey")) {
+ if (!value)
+ die("user.signingkey without value");
+ strlcpy(signingkey, value, sizeof signingkey);
+ return 0;
+ }
+
+ return git_default_config(var, value);
+}
+
+int cmd_tag(int argc, const char **argv, const char *prefix)
+{
+ char buffer[4096];
+ unsigned char object[20], prev[20], result[20];
+ int annotate = 0, is_signed = 0, force = 0, lines = 0;
+ const char *message = NULL;
+ char ref[PATH_MAX];
+ const char *object_ref, *tag;
+ int i, body, header, total, fd;
+ enum object_type type;
+ struct ref_lock *lock;
+
+ git_config(git_tag_config);
+
+ for (i = 1; i < argc; i++) {
+ const char *arg = argv[i];
+
+ if (arg[0] != '-')
+ break;
+ if (!strcmp(arg, "-a")) {
+ annotate = 1;
+ continue;
+ }
+ if (!strcmp(arg, "-s")) {
+ annotate = 1;
+ is_signed = 1;
+ continue;
+ }
+ if (!strcmp(arg, "-f")) {
+ force = 1;
+ continue;
+ }
+ if (!strcmp(arg, "-n")) {
+ if (i + 1 == argc || *argv[i + 1] == '-')
+ /* no argument */
+ lines = 1;
+ else
+ /* FIXME, fallback to 1 on invalid integer */
+ lines = atoi(argv[i + 1]);
+ continue;
+ }
+ if (!strcmp(arg, "-m")) {
+ annotate = 1;
+ i++;
+ if (i == argc)
+ die("option -m needs an argument.");
+ message = argv[i];
+ continue;
+ }
+ if (!strcmp(arg, "-F")) {
+ annotate = 1;
+ i++;
+ if (i == argc)
+ die("option -F needs an argument.");
+
+ fd = open(argv[i], O_RDONLY);
+ if (fd < 0)
+ die("cannot open %s", argv[1]);
+
+ message = xmalloc(4096);
+ if (read_in_full(fd, (char *) message, 4096) < 0)
+ die("cannot read %s", argv[1]);
+ continue;
+ }
+ if (!strcmp(arg, "-u")) {
+ annotate = 1;
+ is_signed = 1;
+ i++;
+ if (i == argc)
+ die("option -u needs an argument.");
+ strlcpy(signingkey, argv[i], sizeof signingkey);
+ continue;
+ }
+ if (!strcmp(arg, "-l")) {
+ return list_tags(argv[i + 1]);
+ }
+ if (!strcmp(arg, "-d")) {
+ return delete_tags(argv + i + 1);
+ }
+ if (!strcmp(arg, "-v")) {
+ return verify_tags(argv + i + 1);
+ }
+ usage(builtin_tag_usage);
+ }
+
+ if (i == argc)
+ return list_tags(NULL);
+ tag = argv[i++];
+
+ if (i < argc)
+ object_ref = argv[i];
+ else
+ object_ref = "HEAD";
+
+ if (get_sha1(object_ref, object))
+ die("Failed to resolve '%s' as a valid ref.", object_ref);
+
+ if (snprintf(ref, sizeof ref, "refs/tags/%s", tag) > sizeof ref)
+ die("tag '%s' too long.", tag);
+ if (check_ref_format(ref))
+ die("'%s' is not a valid tag name.", tag);
+ if (resolve_ref(ref, prev, 1, NULL)) {
+ if (!force)
+ die("tag '%s' already exists", tag);
+ } else {
+ hashclr(prev);
+ }
+
+ type = sha1_object_info(object, NULL);
+ if (type <= 0)
+ die("bad object type.");
+
+ header = snprintf(buffer, sizeof buffer,
+ "object %s\n"
+ "type %s\n"
+ "tag %s\n"
+ "tagger %s\n\n",
+ sha1_to_hex(object),
+ typename(type),
+ tag,
+ git_committer_info(1));
+
+ if (annotate && message == NULL)
+ body = launch_editor(git_path("TAGMSG"), tag_template,
+ buffer + header, sizeof buffer - header);
+ else if (annotate)
+ body = snprintf(buffer + header, sizeof buffer - header,
+ "%s\n", message);
+ else
+ body = 0;
+
+ if (annotate && body == 0)
+ die("no tag message?");
+
+ if (header + body > sizeof buffer)
+ die("tag message too big.");
+
+ if (is_signed)
+ total = do_sign(buffer, header + body, sizeof buffer);
+ else
+ total = header + body;
+
+ if (write_sha1_file(buffer, total, tag_type, result) < 0)
+ die("unable to write tag file");
+
+ lock = lock_any_ref_for_update(ref, prev, 0);
+ if (!lock)
+ die("%s: cannot lock the ref", ref);
+ if (write_ref_sha1(lock, result, NULL) < 0)
+ die("%s: cannot update the ref", ref);
+
+ return 0;
+}
diff --git a/builtin.h b/builtin.h
index 39290d1..91166e1 100644
--- a/builtin.h
+++ b/builtin.h
@@ -72,6 +72,7 @@ extern int cmd_show(int argc, const char **argv, const char *prefix);
extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
+extern int cmd_tag(int argc, const char **argv, const char *prefix);
extern int cmd_tar_tree(int argc, const char **argv, const char *prefix);
extern int cmd_unpack_objects(int argc, const char **argv, const char *prefix);
extern int cmd_update_index(int argc, const char **argv, const char *prefix);
diff --git a/git-tag.sh b/git-tag.sh
deleted file mode 100755
index 37cee97..0000000
--- a/git-tag.sh
+++ /dev/null
@@ -1,183 +0,0 @@
-#!/bin/sh
-# Copyright (c) 2005 Linus Torvalds
-
-USAGE='[-n [<num>]] -l [<pattern>] | [-a | -s | -u <key-id>] [-f | -d | -v] [-m <msg>] <tagname> [<head>]'
-SUBDIRECTORY_OK='Yes'
-. git-sh-setup
-
-message_given=
-annotate=
-signed=
-force=
-message=
-username=
-list=
-verify=
-LINES=0
-while case "$#" in 0) break ;; esac
-do
- case "$1" in
- -a)
- annotate=1
- ;;
- -s)
- annotate=1
- signed=1
- ;;
- -f)
- force=1
- ;;
- -n)
- case $2 in
- -*) LINES=1 # no argument
- ;;
- *) shift
- LINES=$(expr "$1" : '\([0-9]*\)')
- [ -z "$LINES" ] && LINES=1 # 1 line is default when -n is used
- ;;
- esac
- ;;
- -l)
- list=1
- shift
- PATTERN="$1" # select tags by shell pattern, not re
- git rev-parse --symbolic --tags | sort |
- while read TAG
- do
- case "$TAG" in
- *$PATTERN*) ;;
- *) continue ;;
- esac
- [ "$LINES" -le 0 ] && { echo "$TAG"; continue ;}
- OBJTYPE=$(git cat-file -t "$TAG")
- case $OBJTYPE in
- tag) ANNOTATION=$(git cat-file tag "$TAG" |
- sed -e '1,/^$/d' \
- -e '/^-----BEGIN PGP SIGNATURE-----$/Q' )
- printf "%-15s %s\n" "$TAG" "$ANNOTATION" |
- sed -e '2,$s/^/ /' \
- -e "${LINES}q"
- ;;
- *) echo "$TAG"
- ;;
- esac
- done
- ;;
- -m)
- annotate=1
- shift
- message="$1"
- if test "$#" = "0"; then
- die "error: option -m needs an argument"
- else
- message_given=1
- fi
- ;;
- -F)
- annotate=1
- shift
- if test "$#" = "0"; then
- die "error: option -F needs an argument"
- else
- message="$(cat "$1")"
- message_given=1
- fi
- ;;
- -u)
- annotate=1
- signed=1
- shift
- username="$1"
- ;;
- -d)
- shift
- had_error=0
- for tag
- do
- cur=$(git-show-ref --verify --hash -- "refs/tags/$tag") || {
- echo >&2 "Seriously, what tag are you talking about?"
- had_error=1
- continue
- }
- git-update-ref -m 'tag: delete' -d "refs/tags/$tag" "$cur" || {
- had_error=1
- continue
- }
- echo "Deleted tag $tag."
- done
- exit $had_error
- ;;
- -v)
- shift
- tag_name="$1"
- tag=$(git-show-ref --verify --hash -- "refs/tags/$tag_name") ||
- die "Seriously, what tag are you talking about?"
- git-verify-tag -v "$tag"
- exit $?
- ;;
- -*)
- usage
- ;;
- *)
- break
- ;;
- esac
- shift
-done
-
-[ -n "$list" ] && exit 0
-
-name="$1"
-[ "$name" ] || usage
-prev=0000000000000000000000000000000000000000
-if git-show-ref --verify --quiet -- "refs/tags/$name"
-then
- test -n "$force" || die "tag '$name' already exists"
- prev=`git rev-parse "refs/tags/$name"`
-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
-tagger=$(git-var GIT_COMMITTER_IDENT) || exit 1
-
-test -n "$username" ||
- username=$(git-repo-config user.signingkey) ||
- username=$(expr "z$tagger" : 'z\(.*>\)')
-
-trap 'rm -f "$GIT_DIR"/TAG_TMP* "$GIT_DIR"/TAG_FINALMSG "$GIT_DIR"/TAG_EDITMSG' 0
-
-if [ "$annotate" ]; then
- if [ -z "$message_given" ]; then
- ( echo "#"
- echo "# Write a tag message"
- echo "#" ) > "$GIT_DIR"/TAG_EDITMSG
- ${VISUAL:-${EDITOR:-vi}} "$GIT_DIR"/TAG_EDITMSG || exit
- else
- printf '%s\n' "$message" >"$GIT_DIR"/TAG_EDITMSG
- fi
-
- grep -v '^#' <"$GIT_DIR"/TAG_EDITMSG |
- git-stripspace >"$GIT_DIR"/TAG_FINALMSG
-
- [ -s "$GIT_DIR"/TAG_FINALMSG -o -n "$message_given" ] || {
- echo >&2 "No tag message?"
- exit 1
- }
-
- ( printf 'object %s\ntype %s\ntag %s\ntagger %s\n\n' \
- "$object" "$type" "$name" "$tagger";
- cat "$GIT_DIR"/TAG_FINALMSG ) >"$GIT_DIR"/TAG_TMP
- rm -f "$GIT_DIR"/TAG_TMP.asc "$GIT_DIR"/TAG_FINALMSG
- if [ "$signed" ]; then
- gpg -bsa -u "$username" "$GIT_DIR"/TAG_TMP &&
- cat "$GIT_DIR"/TAG_TMP.asc >>"$GIT_DIR"/TAG_TMP ||
- die "failed to sign the tag with GPG."
- fi
- object=$(git-mktag < "$GIT_DIR"/TAG_TMP)
-fi
-
-git update-ref "refs/tags/$name" "$object" "$prev"
-
diff --git a/git.c b/git.c
index 29b55a1..c9c20fb 100644
--- a/git.c
+++ b/git.c
@@ -285,6 +285,7 @@ static void handle_internal_command(int argc, const char **argv, char **envp)
{ "show", cmd_show, RUN_SETUP | USE_PAGER },
{ "stripspace", cmd_stripspace },
{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
+ { "tag", cmd_tag, RUN_SETUP },
{ "tar-tree", cmd_tar_tree },
{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
{ "update-index", cmd_update_index, RUN_SETUP },
--
1.5.2.1.134.g447c2
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH] Port git-tag.sh to C.
2007-06-08 21:38 Kristian Høgsberg
@ 2007-06-08 21:51 ` Johannes Schindelin
2007-06-08 22:05 ` Kristian Høgsberg
2007-06-08 22:39 ` Matthijs Melchior
0 siblings, 2 replies; 21+ messages in thread
From: Johannes Schindelin @ 2007-06-08 21:51 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
[-- Attachment #1: Type: TEXT/PLAIN, Size: 376 bytes --]
Hi,
On Fri, 8 Jun 2007, Kristian Høgsberg wrote:
> A more or less straight-forward port of git-tag.sh to C.
It is somewhat unfortunate that you did not say that you were working on
this stuff; we have a Google Summer of Code project going on, which tries
to port many scripts to builtins.
As it happens, I am working with jasam on exactly the same script.
Pity,
Dscho
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Port git-tag.sh to C.
2007-06-08 21:51 ` Johannes Schindelin
@ 2007-06-08 22:05 ` Kristian Høgsberg
2007-06-08 22:07 ` Johannes Schindelin
2007-06-08 22:36 ` Carlos Rica
2007-06-08 22:39 ` Matthijs Melchior
1 sibling, 2 replies; 21+ messages in thread
From: Kristian Høgsberg @ 2007-06-08 22:05 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
On Fri, 2007-06-08 at 22:51 +0100, Johannes Schindelin wrote:
> Hi,
>
> On Fri, 8 Jun 2007, Kristian Høgsberg wrote:
>
> > A more or less straight-forward port of git-tag.sh to C.
>
> It is somewhat unfortunate that you did not say that you were working on
> this stuff; we have a Google Summer of Code project going on, which tries
> to port many scripts to builtins.
Ugh, yeah... wasted effort... I didn't see anything about the SoC
project on the list and I didn't pre-announce my work because I only
spent little more than half a day...
> As it happens, I am working with jasam on exactly the same script.
Hm... not sure how to resolve this. FWIW, I'm going to send an updated
version of the patch that should also pass the test suite. The previous
version always only creates annotated tags.
Kristian
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Port git-tag.sh to C.
2007-06-08 22:05 ` Kristian Høgsberg
@ 2007-06-08 22:07 ` Johannes Schindelin
2007-06-08 22:36 ` Carlos Rica
1 sibling, 0 replies; 21+ messages in thread
From: Johannes Schindelin @ 2007-06-08 22:07 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=utf-8, Size: 430 bytes --]
Hi,
On Fri, 8 Jun 2007, Kristian Høgsberg wrote:
> FWIW, I'm going to send an updated version of the patch that should also
> pass the test suite. The previous version always only creates annotated
> tags.
As it happens, jasam also worked on a relatively extensive test script.
You might want to test it:
http://repo.or.cz/w/git/builtin-gsoc.git
Branch is "builtin-tag", the test script is t/t7400-tag.sh.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Port git-tag.sh to C.
2007-06-08 22:05 ` Kristian Høgsberg
2007-06-08 22:07 ` Johannes Schindelin
@ 2007-06-08 22:36 ` Carlos Rica
1 sibling, 0 replies; 21+ messages in thread
From: Carlos Rica @ 2007-06-08 22:36 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: Johannes Schindelin, git
2007/6/9, Kristian Høgsberg <krh@redhat.com>:
> Ugh, yeah... wasted effort... I didn't see anything about the SoC
> project on the list and I didn't pre-announce my work because I only
> spent little more than half a day...
Sorry, it should have been announced before this could happen.
> > As it happens, I am working with jasam on exactly the same script.
We were doing a conservative work to get exactly the same options
and behaviour that git-tag.sh does. The work is now almost finished,
so you could help us comparing the two versions to see how could
end with a better git-tag. It would be fantastic if we could point what
parts of my code can be enhanced with those of yours.
My test suite can also be run against the git-tag.sh code now, you
can check check your code with it. Feel free to say everything you want
about it!
Carlos
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Port git-tag.sh to C.
2007-06-08 21:51 ` Johannes Schindelin
2007-06-08 22:05 ` Kristian Høgsberg
@ 2007-06-08 22:39 ` Matthijs Melchior
1 sibling, 0 replies; 21+ messages in thread
From: Matthijs Melchior @ 2007-06-08 22:39 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Kristian Høgsberg, git
Hi,
Johannes Schindelin wrote:
> Hi,
>
> On Fri, 8 Jun 2007, Kristian Høgsberg wrote:
>
>> A more or less straight-forward port of git-tag.sh to C.
>
> It is somewhat unfortunate that you did not say that you were working on
> this stuff; we have a Google Summer of Code project going on, which tries
> to port many scripts to builtins.
>
> As it happens, I am working with jasam on exactly the same script.
>
> Pity,
> Dscho
Please consider the idea of showing (part of) the tag annotation with
git-tag as well.
See message archived at:
http://permalink.gmane.org/gmane.comp.version-control.git/48962
Thanks,
Matthijs Melchior.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH] Port git-tag.sh to C.
@ 2007-06-08 22:45 Kristian Høgsberg
2007-06-09 1:58 ` Carlos Rica
2007-06-09 18:26 ` Junio C Hamano
0 siblings, 2 replies; 21+ messages in thread
From: Kristian Høgsberg @ 2007-06-08 22:45 UTC (permalink / raw)
To: git; +Cc: Kristian Høgsberg
From: Kristian Høgsberg <krh@redhat.com>
A more or less straight-forward port of git-tag.sh to C.
Signed-off-by: Kristian Høgsberg <krh@redhat.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
Ok, here's an updated version that passes the test suite. Johannes, I
leave it to you and jasam to merge the bits you find useful, but as
far as I see it, this conversion is complete, and there's enough other
shell scripts to port. My port doesn't pass jasam's test suite, it
looks like he is expecting the -l glob to be a regexp, but the
git-tag.sh I started from used shell globs.
Anyways, it'd be nice if you or jasam could keep the list a little
more in the loop with the SoC changes, it is where most of the
development happens, after all. What's next on your list?
cheers,
Kristian
Makefile | 3 +-
builtin-tag.c | 371 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
builtin.h | 1 +
git-tag.sh | 183 ----------------------------
git.c | 1 +
5 files changed, 375 insertions(+), 184 deletions(-)
create mode 100644 builtin-tag.c
delete mode 100755 git-tag.sh
diff --git a/Makefile b/Makefile
index 0f75955..bb1bed1 100644
--- a/Makefile
+++ b/Makefile
@@ -205,7 +205,7 @@ SCRIPT_SH = \
git-pull.sh git-rebase.sh \
git-repack.sh git-request-pull.sh git-reset.sh \
git-sh-setup.sh \
- git-tag.sh git-verify-tag.sh \
+ git-verify-tag.sh \
git-am.sh \
git-merge.sh git-merge-stupid.sh git-merge-octopus.sh \
git-merge-resolve.sh git-merge-ours.sh \
@@ -372,6 +372,7 @@ BUILTIN_OBJS = \
builtin-show-branch.o \
builtin-stripspace.o \
builtin-symbolic-ref.o \
+ builtin-tag.o \
builtin-tar-tree.o \
builtin-unpack-objects.o \
builtin-update-index.o \
diff --git a/builtin-tag.c b/builtin-tag.c
new file mode 100644
index 0000000..26035f5
--- /dev/null
+++ b/builtin-tag.c
@@ -0,0 +1,371 @@
+/*
+ * Builtin "git tag"
+ *
+ * Copyright (c) 2007 Kristian Høgsberg <krh@redhat.com>
+ * Based on git-tag.sh and mktag.c by Linus Torvalds.
+ */
+
+#include "cache.h"
+#include "refs.h"
+#include "commit.h"
+#include "builtin.h"
+#include "tag.h"
+#include "run-command.h"
+
+static const char builtin_tag_usage[] =
+ "git-tag [-n [<num>]] -l [<pattern>] | [-a | -s | -u <key-id>] [-f | -d | -v] [-m <msg>] <tagname> [<head>]";
+
+static char signingkey[1000];
+
+static int launch_editor(const char *path, const char *template,
+ char *buffer, size_t size)
+{
+ struct child_process child;
+ const char *editor;
+ const char *args[3];
+ char *eol;
+ int len, fd, blank_lines, i, j;
+
+ fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0644);
+ if (fd < 0)
+ die("could not create file %s.", path);
+
+ len = strlen(template);
+ write_or_die(fd, template, len);
+ close(fd);
+
+ editor = getenv("VISUAL");
+ if (!editor)
+ editor = getenv("EDITOR");
+ if (!editor)
+ editor = "vi";
+
+ memset(&child, 0, sizeof(child));
+ child.argv = args;
+ args[0] = editor;
+ args[1] = path;
+ args[2] = NULL;
+
+ if (run_command(&child))
+ die("could not launch editor %s.", editor);
+
+ fd = open(path, O_RDONLY, 0644);
+ if (fd == -1)
+ die("could not read %s.", path);
+ len = read_in_full(fd, buffer, size);
+ if (len < 0)
+ die("failed to read '%s', %m", path);
+ close(fd);
+
+ blank_lines = 1;
+ for (i = 0, j = 0; i < len; i++) {
+ if (blank_lines > 0 && buffer[i] == '#') {
+ eol = strchr(buffer + i, '\n');
+ if (!eol)
+ break;
+
+ i = eol - buffer;
+ continue;
+ }
+
+ if (buffer[i] == '\n') {
+ blank_lines++;
+ if (blank_lines > 1)
+ continue;
+ } else {
+ if (blank_lines > 2)
+ buffer[j++] = '\n';
+ blank_lines = 0;
+ }
+
+ buffer[j++] = buffer[i];
+ }
+
+ if (buffer[j - 1] != '\n')
+ buffer[j++] = '\n';
+
+ unlink(path);
+
+ return j;
+}
+
+static int show_reference(const char *refname, const unsigned char *sha1,
+ int flag, void *cb_data)
+{
+ const char *pattern = cb_data;
+
+ if (pattern == NULL || !fnmatch(pattern, refname, 0))
+ printf("%s\n", refname);
+
+ return 0;
+}
+
+static int list_tags(const char *pattern)
+{
+ for_each_tag_ref(show_reference, (void *) pattern);
+
+ return 0;
+}
+
+
+static int delete_tags(const char **argv)
+{
+ const char **p;
+ char ref[PATH_MAX];
+ int had_error = 0;
+ unsigned char sha1[20];
+
+ for (p = argv; *p; p++) {
+ if (snprintf(ref, sizeof ref, "refs/tags/%s", *p) > sizeof ref)
+ die("tag name '%s' too long.", *p);
+ if (!resolve_ref(ref, sha1, 1, NULL)) {
+ fprintf(stderr, "tag '%s' not found.\n", *p);
+ had_error = 1;
+ continue;
+ }
+
+ if (!delete_ref(ref, sha1))
+ printf("Deleted tag '%s'\n", *p);
+ }
+
+ return had_error;
+}
+
+static int verify_tags(const char **argv)
+{
+ const char **p;
+ char ref[PATH_MAX];
+ int had_error = 0;
+ unsigned char sha1[20];
+
+ for (p = argv; *p; p++) {
+ if (snprintf(ref, sizeof ref, "refs/tags/%s", *p) > sizeof ref)
+ die("tag name '%s' too long.", *p);
+
+ if (!resolve_ref(ref, sha1, 1, NULL)) {
+ fprintf(stderr, "tag '%s' not found.\n", *p);
+ had_error = 1;
+ continue;
+ }
+
+ printf("FIXME: verify tag '%s'\n", *p);
+ }
+
+ return had_error;
+}
+
+static int do_sign(char *buffer, size_t size, size_t max)
+{
+ struct child_process gpg;
+ const char *args[5];
+ char *bracket;
+ int len;
+
+ if (signingkey[0] == '\0') {
+ strlcpy(signingkey, git_committer_info(1), sizeof signingkey);
+ bracket = strchr(signingkey, '>');
+ if (bracket)
+ bracket[1] = '\0';
+ }
+
+ memset(&gpg, 0, sizeof(gpg));
+ gpg.argv = args;
+ gpg.in = -1;
+ gpg.out = -1;
+ args[0] = "gpg";
+ args[1] = "-bsa";
+ args[2] = "-u";
+ args[3] = signingkey;
+ args[4] = NULL;
+
+ if (start_command(&gpg))
+ die("could not run gpg.");
+
+ write_or_die(gpg.in, buffer, size);
+ close(gpg.in);
+ gpg.close_in = 0;
+ len = read_in_full(gpg.out, buffer + size, max - size);
+
+ finish_command(&gpg);
+
+ return size + len;
+}
+
+static const char tag_template[] =
+ "\n"
+ "#\n"
+ "# Write a tag message\n"
+ "#\n";
+
+static int git_tag_config(const char *var, const char *value)
+{
+ if (!strcmp(var, "user.signingkey")) {
+ if (!value)
+ die("user.signingkey without value");
+ strlcpy(signingkey, value, sizeof signingkey);
+ return 0;
+ }
+
+ return git_default_config(var, value);
+}
+
+static void create_tag(const unsigned char *object, const char *tag,
+ const char *message, int sign, unsigned char *result)
+{
+ enum object_type type;
+ char buffer[4096];
+ int header, body, total;
+
+ type = sha1_object_info(object, NULL);
+ if (type <= 0)
+ die("bad object type.");
+
+ header = snprintf(buffer, sizeof buffer,
+ "object %s\n"
+ "type %s\n"
+ "tag %s\n"
+ "tagger %s\n\n",
+ sha1_to_hex(object),
+ typename(type),
+ tag,
+ git_committer_info(1));
+
+ if (message == NULL)
+ body = launch_editor(git_path("TAGMSG"), tag_template,
+ buffer + header, sizeof buffer - header);
+ else
+ body = snprintf(buffer + header, sizeof buffer - header,
+ "%s\n", message);
+
+ if (body == 0)
+ die("no tag message?");
+
+ if (header + body > sizeof buffer)
+ die("tag message too big.");
+
+ if (sign)
+ total = do_sign(buffer, header + body, sizeof buffer);
+ else
+ total = header + body;
+
+ if (write_sha1_file(buffer, total, tag_type, result) < 0)
+ die("unable to write tag file");
+}
+
+int cmd_tag(int argc, const char **argv, const char *prefix)
+{
+ unsigned char object[20], prev[20], result[20];
+ int annotate = 0, sign = 0, force = 0, lines = 0;
+ const char *message = NULL;
+ char ref[PATH_MAX];
+ const char *object_ref, *tag;
+ int i, fd;
+ struct ref_lock *lock;
+
+ git_config(git_tag_config);
+
+ for (i = 1; i < argc; i++) {
+ const char *arg = argv[i];
+
+ if (arg[0] != '-')
+ break;
+ if (!strcmp(arg, "-a")) {
+ annotate = 1;
+ continue;
+ }
+ if (!strcmp(arg, "-s")) {
+ annotate = 1;
+ sign = 1;
+ continue;
+ }
+ if (!strcmp(arg, "-f")) {
+ force = 1;
+ continue;
+ }
+ if (!strcmp(arg, "-n")) {
+ if (i + 1 == argc || *argv[i + 1] == '-')
+ /* no argument */
+ lines = 1;
+ else
+ /* FIXME, fallback to 1 on invalid integer */
+ lines = atoi(argv[i + 1]);
+ continue;
+ }
+ if (!strcmp(arg, "-m")) {
+ annotate = 1;
+ i++;
+ if (i == argc)
+ die("option -m needs an argument.");
+ message = argv[i];
+ continue;
+ }
+ if (!strcmp(arg, "-F")) {
+ annotate = 1;
+ i++;
+ if (i == argc)
+ die("option -F needs an argument.");
+
+ fd = open(argv[i], O_RDONLY);
+ if (fd < 0)
+ die("cannot open %s", argv[1]);
+
+ message = xmalloc(4096);
+ if (read_in_full(fd, (char *) message, 4096) < 0)
+ die("cannot read %s", argv[1]);
+ continue;
+ }
+ if (!strcmp(arg, "-u")) {
+ annotate = 1;
+ sign = 1;
+ i++;
+ if (i == argc)
+ die("option -u needs an argument.");
+ strlcpy(signingkey, argv[i], sizeof signingkey);
+ continue;
+ }
+ if (!strcmp(arg, "-l")) {
+ return list_tags(argv[i + 1]);
+ }
+ if (!strcmp(arg, "-d")) {
+ return delete_tags(argv + i + 1);
+ }
+ if (!strcmp(arg, "-v")) {
+ return verify_tags(argv + i + 1);
+ }
+ usage(builtin_tag_usage);
+ }
+
+ if (i == argc)
+ return list_tags(NULL);
+ tag = argv[i++];
+
+ if (i < argc)
+ object_ref = argv[i];
+ else
+ object_ref = "HEAD";
+
+ if (get_sha1(object_ref, object))
+ die("Failed to resolve '%s' as a valid ref.", object_ref);
+
+ if (snprintf(ref, sizeof ref, "refs/tags/%s", tag) > sizeof ref)
+ die("tag '%s' too long.", tag);
+ if (check_ref_format(ref))
+ die("'%s' is not a valid tag name.", tag);
+ if (resolve_ref(ref, prev, 1, NULL)) {
+ if (!force)
+ die("tag '%s' already exists", tag);
+ } else {
+ hashclr(prev);
+ }
+
+ if (annotate)
+ create_tag(object, tag, message, sign, object);
+
+ lock = lock_any_ref_for_update(ref, prev, 0);
+ if (!lock)
+ die("%s: cannot lock the ref", ref);
+ if (write_ref_sha1(lock, object, NULL) < 0)
+ die("%s: cannot update the ref", ref);
+
+ return 0;
+}
diff --git a/builtin.h b/builtin.h
index 39290d1..91166e1 100644
--- a/builtin.h
+++ b/builtin.h
@@ -72,6 +72,7 @@ extern int cmd_show(int argc, const char **argv, const char *prefix);
extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
+extern int cmd_tag(int argc, const char **argv, const char *prefix);
extern int cmd_tar_tree(int argc, const char **argv, const char *prefix);
extern int cmd_unpack_objects(int argc, const char **argv, const char *prefix);
extern int cmd_update_index(int argc, const char **argv, const char *prefix);
diff --git a/git-tag.sh b/git-tag.sh
deleted file mode 100755
index 37cee97..0000000
--- a/git-tag.sh
+++ /dev/null
@@ -1,183 +0,0 @@
-#!/bin/sh
-# Copyright (c) 2005 Linus Torvalds
-
-USAGE='[-n [<num>]] -l [<pattern>] | [-a | -s | -u <key-id>] [-f | -d | -v] [-m <msg>] <tagname> [<head>]'
-SUBDIRECTORY_OK='Yes'
-. git-sh-setup
-
-message_given=
-annotate=
-signed=
-force=
-message=
-username=
-list=
-verify=
-LINES=0
-while case "$#" in 0) break ;; esac
-do
- case "$1" in
- -a)
- annotate=1
- ;;
- -s)
- annotate=1
- signed=1
- ;;
- -f)
- force=1
- ;;
- -n)
- case $2 in
- -*) LINES=1 # no argument
- ;;
- *) shift
- LINES=$(expr "$1" : '\([0-9]*\)')
- [ -z "$LINES" ] && LINES=1 # 1 line is default when -n is used
- ;;
- esac
- ;;
- -l)
- list=1
- shift
- PATTERN="$1" # select tags by shell pattern, not re
- git rev-parse --symbolic --tags | sort |
- while read TAG
- do
- case "$TAG" in
- *$PATTERN*) ;;
- *) continue ;;
- esac
- [ "$LINES" -le 0 ] && { echo "$TAG"; continue ;}
- OBJTYPE=$(git cat-file -t "$TAG")
- case $OBJTYPE in
- tag) ANNOTATION=$(git cat-file tag "$TAG" |
- sed -e '1,/^$/d' \
- -e '/^-----BEGIN PGP SIGNATURE-----$/Q' )
- printf "%-15s %s\n" "$TAG" "$ANNOTATION" |
- sed -e '2,$s/^/ /' \
- -e "${LINES}q"
- ;;
- *) echo "$TAG"
- ;;
- esac
- done
- ;;
- -m)
- annotate=1
- shift
- message="$1"
- if test "$#" = "0"; then
- die "error: option -m needs an argument"
- else
- message_given=1
- fi
- ;;
- -F)
- annotate=1
- shift
- if test "$#" = "0"; then
- die "error: option -F needs an argument"
- else
- message="$(cat "$1")"
- message_given=1
- fi
- ;;
- -u)
- annotate=1
- signed=1
- shift
- username="$1"
- ;;
- -d)
- shift
- had_error=0
- for tag
- do
- cur=$(git-show-ref --verify --hash -- "refs/tags/$tag") || {
- echo >&2 "Seriously, what tag are you talking about?"
- had_error=1
- continue
- }
- git-update-ref -m 'tag: delete' -d "refs/tags/$tag" "$cur" || {
- had_error=1
- continue
- }
- echo "Deleted tag $tag."
- done
- exit $had_error
- ;;
- -v)
- shift
- tag_name="$1"
- tag=$(git-show-ref --verify --hash -- "refs/tags/$tag_name") ||
- die "Seriously, what tag are you talking about?"
- git-verify-tag -v "$tag"
- exit $?
- ;;
- -*)
- usage
- ;;
- *)
- break
- ;;
- esac
- shift
-done
-
-[ -n "$list" ] && exit 0
-
-name="$1"
-[ "$name" ] || usage
-prev=0000000000000000000000000000000000000000
-if git-show-ref --verify --quiet -- "refs/tags/$name"
-then
- test -n "$force" || die "tag '$name' already exists"
- prev=`git rev-parse "refs/tags/$name"`
-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
-tagger=$(git-var GIT_COMMITTER_IDENT) || exit 1
-
-test -n "$username" ||
- username=$(git-repo-config user.signingkey) ||
- username=$(expr "z$tagger" : 'z\(.*>\)')
-
-trap 'rm -f "$GIT_DIR"/TAG_TMP* "$GIT_DIR"/TAG_FINALMSG "$GIT_DIR"/TAG_EDITMSG' 0
-
-if [ "$annotate" ]; then
- if [ -z "$message_given" ]; then
- ( echo "#"
- echo "# Write a tag message"
- echo "#" ) > "$GIT_DIR"/TAG_EDITMSG
- ${VISUAL:-${EDITOR:-vi}} "$GIT_DIR"/TAG_EDITMSG || exit
- else
- printf '%s\n' "$message" >"$GIT_DIR"/TAG_EDITMSG
- fi
-
- grep -v '^#' <"$GIT_DIR"/TAG_EDITMSG |
- git-stripspace >"$GIT_DIR"/TAG_FINALMSG
-
- [ -s "$GIT_DIR"/TAG_FINALMSG -o -n "$message_given" ] || {
- echo >&2 "No tag message?"
- exit 1
- }
-
- ( printf 'object %s\ntype %s\ntag %s\ntagger %s\n\n' \
- "$object" "$type" "$name" "$tagger";
- cat "$GIT_DIR"/TAG_FINALMSG ) >"$GIT_DIR"/TAG_TMP
- rm -f "$GIT_DIR"/TAG_TMP.asc "$GIT_DIR"/TAG_FINALMSG
- if [ "$signed" ]; then
- gpg -bsa -u "$username" "$GIT_DIR"/TAG_TMP &&
- cat "$GIT_DIR"/TAG_TMP.asc >>"$GIT_DIR"/TAG_TMP ||
- die "failed to sign the tag with GPG."
- fi
- object=$(git-mktag < "$GIT_DIR"/TAG_TMP)
-fi
-
-git update-ref "refs/tags/$name" "$object" "$prev"
-
diff --git a/git.c b/git.c
index 29b55a1..c9c20fb 100644
--- a/git.c
+++ b/git.c
@@ -285,6 +285,7 @@ static void handle_internal_command(int argc, const char **argv, char **envp)
{ "show", cmd_show, RUN_SETUP | USE_PAGER },
{ "stripspace", cmd_stripspace },
{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
+ { "tag", cmd_tag, RUN_SETUP },
{ "tar-tree", cmd_tar_tree },
{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
{ "update-index", cmd_update_index, RUN_SETUP },
--
1.5.0.6
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH] Port git-tag.sh to C.
2007-06-08 22:45 [PATCH] Port git-tag.sh to C Kristian Høgsberg
@ 2007-06-09 1:58 ` Carlos Rica
2007-06-12 3:28 ` Daniel Barkalow
2007-06-09 18:26 ` Junio C Hamano
1 sibling, 1 reply; 21+ messages in thread
From: Carlos Rica @ 2007-06-09 1:58 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
2007/6/9, Kristian Høgsberg <krh@redhat.com>:
> Ok, here's an updated version that passes the test suite. Johannes, I
> leave it to you and jasam to merge the bits you find useful, but as
> far as I see it, this conversion is complete, and there's enough other
> shell scripts to port. My port doesn't pass jasam's test suite, it
> looks like he is expecting the -l glob to be a regexp, but the
> git-tag.sh I started from used shell globs.
That's a good proposal! I agree with your version for matching tag names,
apart from the fact that the code is more compact and readable that way.
I will review your code comparing it with mine and I will report
everything I feel. However, my desire is to get builtin-tag.c and its
tests t7400-tag.sh complete
in a few days or less, so I would appreciate every other correction
from you in order to get it ready faster.
> Anyways, it'd be nice if you or jasam could keep the list a little
> more in the loop with the SoC changes, it is where most of the
> development happens, after all. What's next on your list?
You're right, the development will be more open than now was, I
usually ask questions in the IRC channel and obviously that's not the
best way to report my work to everybody.
Feel free to choose the script which you need to get replaced first,
or, depending on your urgency, you could ask me for one of them and I
would try to concentrate my efforts on it. Why do you started with
git-tag? For me, it was enough easy to begin with, perhaps you could
have other reasons.
Looking forward to work with you on that,
Carlos
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Port git-tag.sh to C.
2007-06-08 22:45 [PATCH] Port git-tag.sh to C Kristian Høgsberg
2007-06-09 1:58 ` Carlos Rica
@ 2007-06-09 18:26 ` Junio C Hamano
2007-06-09 23:27 ` Kristian Høgsberg
2007-06-10 22:13 ` Jeff King
1 sibling, 2 replies; 21+ messages in thread
From: Junio C Hamano @ 2007-06-09 18:26 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
Kristian Høgsberg <krh@redhat.com> writes:
> Content-Type: TEXT/PLAIN; charset=ISO-8859-1
>
> From: Kristian Høgsberg <krh@redhat.com>
>
> A more or less straight-forward port of git-tag.sh to C.
>
> Signed-off-by: Kristian Høgsberg <krh@redhat.com>
> Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>
I think your name in your commit message is in UTF-8 but munged your
mail was mismarked as iso-8859-1.
> +static int launch_editor(const char *path, const char *template,
> + char *buffer, size_t size)
> +{
It would have been nicer to have this in editor.c or somesuch,
as other commands will be redone in C in the future.
We could do the moving later, but the problem is that later is
conditional: "if we are lucky enough to remember that we already
have this function in builtin-tag when doing so".
> + fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0644);
I would understand an argument to use 0666 (honor umask) or 0600
(this is a temporary file and others have no business looking at
it while an edit is in progress), but I cannot justify 0644.
> + fd = open(path, O_RDONLY, 0644);
Open for reading with mode ;-)?
> + if (fd == -1)
> + die("could not read %s.", path);
> + len = read_in_full(fd, buffer, size);
> + if (len < 0)
> + die("failed to read '%s', %m", path);
> + close(fd);
> +
> + blank_lines = 1;
> + for (i = 0, j = 0; i < len; i++) {
> ...
> + }
> +
> + if (buffer[j - 1] != '\n')
> + buffer[j++] = '\n';
> +
> + unlink(path);
> +
> + return j;
> +}
I really think this function needs to be refactored into three.
* A generic "spawn an editor with this initial seed template,
return the result of editing in memory and also give exit
status of the editor" function that does not take path
parameter (instead perhaps mkstemp a temporary file on your
own);
* A function that does what git-stripspace does in core;
* A function for builtin-tag to use, that calls the above two
and uses the result (e.g. "did the user kill the editor?
does the resulting buffer have any nonempty line?") to decide
what it does.
> +static void create_tag(const unsigned char *object, const char *tag,
> + const char *message, int sign, unsigned char *result)
> +{
> + enum object_type type;
> + char buffer[4096];
> + int header, body, total;
> +
> + type = sha1_object_info(object, NULL);
> + if (type <= 0)
> + die("bad object type.");
> +
> + header = snprintf(buffer, sizeof buffer,
> + "object %s\n"
> + "type %s\n"
> + "tag %s\n"
> + "tagger %s\n\n",
> + sha1_to_hex(object),
> + typename(type),
> + tag,
> + git_committer_info(1));
> +
> + if (message == NULL)
> + body = launch_editor(git_path("TAGMSG"), tag_template,
> + buffer + header, sizeof buffer - header);
> + else
> + body = snprintf(buffer + header, sizeof buffer - header,
> + "%s\n", message);
> +
> + if (body == 0)
> + die("no tag message?");
> +
> + if (header + body > sizeof buffer)
> + die("tag message too big.");
Two issues:
* It used to be a tag had limit of 8kB which was lifted some
time ago; now it is limited to 4kB. Fixing this implies that
the "launch editor and get results in core" function I
mentioned above may need to realloc, and probably the buffer
is better passed as (char *, ulong) pair as done everywhere
else (although we know this is text so you can pass only a
pointer and have the user run strlen() when needed).
* I do not see any validation on the value of "tag". Do we want
to allow passing "" to it? What about "my\ntag"?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Port git-tag.sh to C.
2007-06-09 18:26 ` Junio C Hamano
@ 2007-06-09 23:27 ` Kristian Høgsberg
2007-06-10 12:37 ` Robin Rosenberg
2007-06-10 22:13 ` Jeff King
1 sibling, 1 reply; 21+ messages in thread
From: Kristian Høgsberg @ 2007-06-09 23:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano wrote:
> Kristian Høgsberg <krh@redhat.com> writes:
>
>> Content-Type: TEXT/PLAIN; charset=ISO-8859-1
>>
>> From: Kristian Høgsberg <krh@redhat.com>
>>
>> A more or less straight-forward port of git-tag.sh to C.
>>
>> Signed-off-by: Kristian Høgsberg <krh@redhat.com>
>> Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>
> I think your name in your commit message is in UTF-8 but munged your
> mail was mismarked as iso-8859-1.
That's odd both the email I cc'ed to my redhat.com address and the one I got
on gmail.com through the list have
Content-Type: text/plain; charset=utf-8
and saving the raw message and asking /usr/bin/file, it tells me its
/home/krh/Desktop/hep: UTF-8 Unicode mail text
>> +static int launch_editor(const char *path, const char *template,
>> + char *buffer, size_t size)
>> +{
>
> It would have been nicer to have this in editor.c or somesuch,
> as other commands will be redone in C in the future.
>
> We could do the moving later, but the problem is that later is
> conditional: "if we are lucky enough to remember that we already
> have this function in builtin-tag when doing so".
Yeah, true. I did write it as a generally usable "launch editor" functions,
but I didn't want to move it until there was a second user. Is there anything
else that git-commit that will use this, btw?
>> + fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0644);
>
> I would understand an argument to use 0666 (honor umask) or 0600
> (this is a temporary file and others have no business looking at
> it while an edit is in progress), but I cannot justify 0644.
Oh hehe, yeah, I didn't think much about the permission bits... 0666 sounds fine.
>> + fd = open(path, O_RDONLY, 0644);
>
> Open for reading with mode ;-)?
Even less thinking here :)
>> + if (fd == -1)
>> + die("could not read %s.", path);
>> + len = read_in_full(fd, buffer, size);
>> + if (len < 0)
>> + die("failed to read '%s', %m", path);
>> + close(fd);
>> +
>> + blank_lines = 1;
>> + for (i = 0, j = 0; i < len; i++) {
>> ...
>> + }
>> +
>> + if (buffer[j - 1] != '\n')
>> + buffer[j++] = '\n';
>> +
>> + unlink(path);
>> +
>> + return j;
>> +}
>
> I really think this function needs to be refactored into three.
>
> * A generic "spawn an editor with this initial seed template,
> return the result of editing in memory and also give exit
> status of the editor" function that does not take path
> parameter (instead perhaps mkstemp a temporary file on your
> own);
>
> * A function that does what git-stripspace does in core;
>
> * A function for builtin-tag to use, that calls the above two
> and uses the result (e.g. "did the user kill the editor?
> does the resulting buffer have any nonempty line?") to decide
> what it does.
and that last function should be useful for git-commit too, no? The reason I
didn't split it up was that I don't see any use for the two parts on their
own, and the functions is a total of 70 lines. And wouldn't returning 0 bytes
for either an empty buffer or editor quit be sufficient? Do we need to handle
the two cases differently?
>> +static void create_tag(const unsigned char *object, const char *tag,
>> + const char *message, int sign, unsigned char *result)
>> +{
>> + enum object_type type;
>> + char buffer[4096];
>> + int header, body, total;
>> +
>> + type = sha1_object_info(object, NULL);
>> + if (type <= 0)
>> + die("bad object type.");
>> +
>> + header = snprintf(buffer, sizeof buffer,
>> + "object %s\n"
>> + "type %s\n"
>> + "tag %s\n"
>> + "tagger %s\n\n",
>> + sha1_to_hex(object),
>> + typename(type),
>> + tag,
>> + git_committer_info(1));
>> +
>> + if (message == NULL)
>> + body = launch_editor(git_path("TAGMSG"), tag_template,
>> + buffer + header, sizeof buffer - header);
>> + else
>> + body = snprintf(buffer + header, sizeof buffer - header,
>> + "%s\n", message);
>> +
>> + if (body == 0)
>> + die("no tag message?");
>> +
>> + if (header + body > sizeof buffer)
>> + die("tag message too big.");
>
> Two issues:
>
> * It used to be a tag had limit of 8kB which was lifted some
> time ago; now it is limited to 4kB. Fixing this implies that
> the "launch editor and get results in core" function I
> mentioned above may need to realloc, and probably the buffer
> is better passed as (char *, ulong) pair as done everywhere
> else (although we know this is text so you can pass only a
> pointer and have the user run strlen() when needed).
Oh... read_pipe() reallocs... OK. I took the limit from mktag.c and didn't
realize that the buffer could get realloced. I did wonder about the xmalloc()
for a fixed buffer though...
> * I do not see any validation on the value of "tag". Do we want
> to allow passing "" to it? What about "my\ntag"?
I do
if (check_ref_format(ref))
die("'%s' is not a valid tag name.", tag);
on the ref, which is what git-tag.sh ends up doing, and it catches the two
examples you mention:
[krh@dinky git]$ ./git tag "hello
> world"
fatal: 'hello
world' is not a valid tag name.
[krh@dinky git]$ ./git tag ""
fatal: '' is not a valid tag name.
so I think it's fine.
But I think I'll leave it to Carlos to finish his builtin-tag work, and he can
cherry pick the bits from my patch that work for him. This was more of a
friday afternoon hacking sprint, for my part.
cheers,
Kristian
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Port git-tag.sh to C.
2007-06-09 23:27 ` Kristian Høgsberg
@ 2007-06-10 12:37 ` Robin Rosenberg
2007-06-10 21:36 ` Junio C Hamano
0 siblings, 1 reply; 21+ messages in thread
From: Robin Rosenberg @ 2007-06-10 12:37 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: Junio C Hamano, git
söndag 10 juni 2007 skrev Kristian Høgsberg:
> Junio C Hamano wrote:
> > Kristian Høgsberg <krh@redhat.com> writes:
> >
> >> Content-Type: TEXT/PLAIN; charset=ISO-8859-1
> >>
> >> From: Kristian Høgsberg <krh@redhat.com>
> >>
> >> A more or less straight-forward port of git-tag.sh to C.
> >>
> >> Signed-off-by: Kristian Høgsberg <krh@redhat.com>
> >> Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> >
> > I think your name in your commit message is in UTF-8 but munged your
> > mail was mismarked as iso-8859-1.
>
> That's odd both the email I cc'ed to my redhat.com address and the one I got
> on gmail.com through the list have
>
> Content-Type: text/plain; charset=utf-8
>
> and saving the raw message and asking /usr/bin/file, it tells me its
>
> /home/krh/Desktop/hep: UTF-8 Unicode mail text
Maybe Junio's mail server has a problem with the impliction of this header line?
"Content-Transfer-Encoding: 8bit" ?
-- robin
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Port git-tag.sh to C.
2007-06-10 12:37 ` Robin Rosenberg
@ 2007-06-10 21:36 ` Junio C Hamano
0 siblings, 0 replies; 21+ messages in thread
From: Junio C Hamano @ 2007-06-10 21:36 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: Kristian Høgsberg, git
Robin Rosenberg <robin.rosenberg.lists@dewire.com> writes:
> söndag 10 juni 2007 skrev Kristian Høgsberg:
>> Junio C Hamano wrote:
>> > Kristian Høgsberg <krh@redhat.com> writes:
>> > ...
>> > I think your name in your commit message is in UTF-8 but munged your
>> > mail was mismarked as iso-8859-1.
>>
>> That's odd both the email I cc'ed to my redhat.com address and the one I got
>> on gmail.com through the list have
>>
>> Content-Type: text/plain; charset=utf-8
>>
>> and saving the raw message and asking /usr/bin/file, it tells me its
>>
>> /home/krh/Desktop/hep: UTF-8 Unicode mail text
>
> Maybe Junio's mail server has a problem with the impliction of this header line?
>
> "Content-Transfer-Encoding: 8bit" ?
The copy of Kristian's message gmane has says:
Content-Type: TEXT/PLAIN; charset=ISO-8859-1
I do not think it is just me.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Port git-tag.sh to C.
2007-06-09 18:26 ` Junio C Hamano
2007-06-09 23:27 ` Kristian Høgsberg
@ 2007-06-10 22:13 ` Jeff King
2007-06-10 22:19 ` Junio C Hamano
1 sibling, 1 reply; 21+ messages in thread
From: Jeff King @ 2007-06-10 22:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kristian Høgsberg, git
On Sat, Jun 09, 2007 at 11:26:48AM -0700, Junio C Hamano wrote:
> > Signed-off-by: Kristian Høgsberg <krh@redhat.com>
> I think your name in your commit message is in UTF-8 but munged your
> mail was mismarked as iso-8859-1.
This is the same git-send-email bug that we fixed recently (missing
MIME-Version header). Just look for the vger "we munged your message"
headers in the version you received.
-Peff
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Port git-tag.sh to C.
2007-06-10 22:13 ` Jeff King
@ 2007-06-10 22:19 ` Junio C Hamano
2007-06-10 22:22 ` Jeff King
0 siblings, 1 reply; 21+ messages in thread
From: Junio C Hamano @ 2007-06-10 22:19 UTC (permalink / raw)
To: Jeff King; +Cc: Kristian Høgsberg, git
Jeff King <peff@peff.net> writes:
> This is the same git-send-email bug that we fixed recently (missing
> MIME-Version header). Just look for the vger "we munged your message"
> headers in the version you received.
Ahh. You are right. The message does have these:
X-Warning: Original message contained 8-bit characters, however during
the SMTP transport session the receiving system did not announce
capability of receiving 8-bit SMTP (RFC 1651-1653), and as this
message does not have MIME headers (RFC 2045-2049) to enable
encoding change, we had very little choice.
X-Warning: We ASSUME it is less harmful to add the MIME headers, and
convert the text to Quoted-Printable, than not to do so,
and to strip the message to 7-bits.. (RFC 1428 Appendix A)
X-Warning: We don't know what character set the user used, thus we had to
write these MIME-headers with our local system default value.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Port git-tag.sh to C.
2007-06-10 22:19 ` Junio C Hamano
@ 2007-06-10 22:22 ` Jeff King
0 siblings, 0 replies; 21+ messages in thread
From: Jeff King @ 2007-06-10 22:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kristian Høgsberg, git
On Sun, Jun 10, 2007 at 03:19:00PM -0700, Junio C Hamano wrote:
> > This is the same git-send-email bug that we fixed recently (missing
> > MIME-Version header). Just look for the vger "we munged your message"
> > headers in the version you received.
>
> Ahh. You are right. The message does have these:
Yes, it's easy for me to tell because I get his messages intact. :) Not
to mention his X-Mailer mentions git-send-email 1.5.0.6.
Kristian, the fix is in git 1.5.1.5 and later, if it's not too much
trouble to upgrade.
-Peff
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Port git-tag.sh to C.
2007-06-09 1:58 ` Carlos Rica
@ 2007-06-12 3:28 ` Daniel Barkalow
2007-06-12 12:41 ` git-fetch, was " Johannes Schindelin
0 siblings, 1 reply; 21+ messages in thread
From: Daniel Barkalow @ 2007-06-12 3:28 UTC (permalink / raw)
To: Carlos Rica; +Cc: =?X-UNKNOWN?Q?Kristian_H=F8gsberg?=, git, Julian Phillips
On Sat, 9 Jun 2007, Carlos Rica wrote:
> Feel free to choose the script which you need to get replaced first,
> or, depending on your urgency, you could ask me for one of them and I
> would try to concentrate my efforts on it. Why do you started with
> git-tag? For me, it was enough easy to begin with, perhaps you could
> have other reasons.
Incidentally, I have been working on fetch, based on Julian Phillips's
version. I'm trying to split out the "how do I communicate with remote
repositories" code, and use it for pushing and ls-remote as well as fetch.
I've got a bunch of not-for-official-history development that you should
look at if you try any of the remote-repository-access scripts.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply [flat|nested] 21+ messages in thread
* git-fetch, was Re: [PATCH] Port git-tag.sh to C.
2007-06-12 3:28 ` Daniel Barkalow
@ 2007-06-12 12:41 ` Johannes Schindelin
2007-06-12 13:29 ` Julian Phillips
0 siblings, 1 reply; 21+ messages in thread
From: Johannes Schindelin @ 2007-06-12 12:41 UTC (permalink / raw)
To: Daniel Barkalow
Cc: Carlos Rica, =?X-UNKNOWN?Q?Kristian_H=F8gsberg?=, git,
Julian Phillips
Hi,
On Mon, 11 Jun 2007, Daniel Barkalow wrote:
> On Sat, 9 Jun 2007, Carlos Rica wrote:
>
> > Feel free to choose the script which you need to get replaced first,
> > or, depending on your urgency, you could ask me for one of them and I
> > would try to concentrate my efforts on it. Why do you started with
> > git-tag? For me, it was enough easy to begin with, perhaps you could
> > have other reasons.
>
> Incidentally, I have been working on fetch, based on Julian Phillips's
> version. I'm trying to split out the "how do I communicate with remote
> repositories" code, and use it for pushing and ls-remote as well as
> fetch. I've got a bunch of not-for-official-history development that you
> should look at if you try any of the remote-repository-access scripts.
How about pushing them onto repo.or.cz as a fork of git.git?
Ciao,
Dscho
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: git-fetch, was Re: [PATCH] Port git-tag.sh to C.
2007-06-12 12:41 ` git-fetch, was " Johannes Schindelin
@ 2007-06-12 13:29 ` Julian Phillips
2007-06-12 16:51 ` Daniel Barkalow
2007-06-12 23:55 ` Daniel Barkalow
0 siblings, 2 replies; 21+ messages in thread
From: Julian Phillips @ 2007-06-12 13:29 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Daniel Barkalow, Carlos Rica, =?X-UNKNOWN?Q?Kristian_H=F8gsberg?=,
git
On Tue, 12 Jun 2007, Johannes Schindelin wrote:
> Hi,
>
> On Mon, 11 Jun 2007, Daniel Barkalow wrote:
>
>> On Sat, 9 Jun 2007, Carlos Rica wrote:
>>
>>> Feel free to choose the script which you need to get replaced first,
>>> or, depending on your urgency, you could ask me for one of them and I
>>> would try to concentrate my efforts on it. Why do you started with
>>> git-tag? For me, it was enough easy to begin with, perhaps you could
>>> have other reasons.
>>
>> Incidentally, I have been working on fetch, based on Julian Phillips's
>> version. I'm trying to split out the "how do I communicate with remote
>> repositories" code, and use it for pushing and ls-remote as well as
>> fetch. I've got a bunch of not-for-official-history development that you
>> should look at if you try any of the remote-repository-access scripts.
>
> How about pushing them onto repo.or.cz as a fork of git.git?
I'd quite like to see these changes too - since I haven't finished the
fetch work yet, and I don't want to duplicate effort ;)
(It seems that the closer I get to a working fetch the less time I spend
on it ... :$)
--
Julian
---
Old robot: I choose to believe what I was programmed to believe.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: git-fetch, was Re: [PATCH] Port git-tag.sh to C.
2007-06-12 13:29 ` Julian Phillips
@ 2007-06-12 16:51 ` Daniel Barkalow
2007-06-12 23:55 ` Daniel Barkalow
1 sibling, 0 replies; 21+ messages in thread
From: Daniel Barkalow @ 2007-06-12 16:51 UTC (permalink / raw)
To: Julian Phillips
Cc: Johannes Schindelin, Carlos Rica,
=?X-UNKNOWN?Q?Kristian_H=F8gsberg?=, git
On Tue, 12 Jun 2007, Julian Phillips wrote:
> On Tue, 12 Jun 2007, Johannes Schindelin wrote:
>
> > How about pushing them onto repo.or.cz as a fork of git.git?
>
> I'd quite like to see these changes too - since I haven't finished the fetch
> work yet, and I don't want to duplicate effort ;)
I'll put it up on my server this evening; I forgot to push the latest
version from my laptop to somewhere I can get it.
> (It seems that the closer I get to a working fetch the less time I spend on it
> ... :$)
My efforts have similarly stagnated, but the GSoC talk reminded me that a
third person could take over. Although the series I just posted adds
branch config parsing, which is helpful for making fetch clean, even if it
doesn't make it closer to working completely.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: git-fetch, was Re: [PATCH] Port git-tag.sh to C.
2007-06-12 13:29 ` Julian Phillips
2007-06-12 16:51 ` Daniel Barkalow
@ 2007-06-12 23:55 ` Daniel Barkalow
2007-06-13 0:27 ` Johannes Schindelin
1 sibling, 1 reply; 21+ messages in thread
From: Daniel Barkalow @ 2007-06-12 23:55 UTC (permalink / raw)
To: Julian Phillips
Cc: Johannes Schindelin, Carlos Rica,
=?X-UNKNOWN?Q?Kristian_H=F8gsberg?=, git
On Tue, 12 Jun 2007, Julian Phillips wrote:
> On Tue, 12 Jun 2007, Johannes Schindelin wrote:
>
> > Hi,
> >
> > On Mon, 11 Jun 2007, Daniel Barkalow wrote:
> >
> > > On Sat, 9 Jun 2007, Carlos Rica wrote:
> > >
> > > > Feel free to choose the script which you need to get replaced first,
> > > > or, depending on your urgency, you could ask me for one of them and I
> > > > would try to concentrate my efforts on it. Why do you started with
> > > > git-tag? For me, it was enough easy to begin with, perhaps you could
> > > > have other reasons.
> > >
> > > Incidentally, I have been working on fetch, based on Julian Phillips's
> > > version. I'm trying to split out the "how do I communicate with remote
> > > repositories" code, and use it for pushing and ls-remote as well as
> > > fetch. I've got a bunch of not-for-official-history development that you
> > > should look at if you try any of the remote-repository-access scripts.
> >
> > How about pushing them onto repo.or.cz as a fork of git.git?
>
> I'd quite like to see these changes too - since I haven't finished the fetch
> work yet, and I don't want to duplicate effort ;)
Okay, git://iabervon.org/~barkalow/git.git builtin-fetch.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: git-fetch, was Re: [PATCH] Port git-tag.sh to C.
2007-06-12 23:55 ` Daniel Barkalow
@ 2007-06-13 0:27 ` Johannes Schindelin
0 siblings, 0 replies; 21+ messages in thread
From: Johannes Schindelin @ 2007-06-13 0:27 UTC (permalink / raw)
To: Daniel Barkalow
Cc: Julian Phillips, Carlos Rica, =?X-UNKNOWN?Q?Kristian_H=F8gsberg?=,
git
Hi,
On Tue, 12 Jun 2007, Daniel Barkalow wrote:
> Okay, git://iabervon.org/~barkalow/git.git builtin-fetch.
Thanks!
Ciao,
Dscho
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2007-06-13 0:30 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-06-08 22:45 [PATCH] Port git-tag.sh to C Kristian Høgsberg
2007-06-09 1:58 ` Carlos Rica
2007-06-12 3:28 ` Daniel Barkalow
2007-06-12 12:41 ` git-fetch, was " Johannes Schindelin
2007-06-12 13:29 ` Julian Phillips
2007-06-12 16:51 ` Daniel Barkalow
2007-06-12 23:55 ` Daniel Barkalow
2007-06-13 0:27 ` Johannes Schindelin
2007-06-09 18:26 ` Junio C Hamano
2007-06-09 23:27 ` Kristian Høgsberg
2007-06-10 12:37 ` Robin Rosenberg
2007-06-10 21:36 ` Junio C Hamano
2007-06-10 22:13 ` Jeff King
2007-06-10 22:19 ` Junio C Hamano
2007-06-10 22:22 ` Jeff King
-- strict thread matches above, loose matches on Subject: below --
2007-06-08 21:38 Kristian Høgsberg
2007-06-08 21:51 ` Johannes Schindelin
2007-06-08 22:05 ` Kristian Høgsberg
2007-06-08 22:07 ` Johannes Schindelin
2007-06-08 22:36 ` Carlos Rica
2007-06-08 22:39 ` Matthijs Melchior
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).