* [PATCH 6/6] DWIM "git add remote ..."
From: Johannes Schindelin @ 2007-12-05 19:03 UTC (permalink / raw)
To: git, gitster
In-Reply-To: <Pine.LNX.4.64.0712051858270.27959@racer.site>
It is wrong to divert to "git remote add" when you typed the
(more English) "git add remote". But is it also pretty convenient.
So implement it, but do not document it ;-)
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin-add.c | 7 +++++++
t/t5505-remote.sh | 6 ++++++
2 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/builtin-add.c b/builtin-add.c
index 5c29cc2..b5b4c2f 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -196,6 +196,13 @@ int cmd_add(int argc, const char **argv, const char *prefix)
const char **pathspec;
struct dir_struct dir;
+ if (argc > 1 && !strcmp(argv[1], "remote") &&
+ access("remote", F_OK)) {
+ argv[1] = argv[0];
+ argv[0] = "remote";
+ return cmd_remote(argc, argv, prefix);
+ }
+
argc = parse_options(argc, argv, builtin_add_options,
builtin_add_usage, 0);
if (patch_interactive)
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 2376e0a..83ed70c 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -147,4 +147,10 @@ test_expect_success 'add --mirror && prune' '
git rev-parse --verify refs/heads/side)
'
+test_expect_success 'add remote' '
+ (cd test &&
+ git add remote -f another-one ../one &&
+ git diff master remotes/another-one/master)
+'
+
test_done
--
1.5.3.7.2157.g9598e
^ permalink raw reply related
* [PATCH 1/6] path-list: add functions to work with unsorted lists
From: Johannes Schindelin @ 2007-12-05 19:05 UTC (permalink / raw)
To: git, gitster
In-Reply-To: <Pine.LNX.4.64.0712051900370.27959@racer.site>
Up to now, path-lists were sorted at all times. But sometimes it
is much more convenient to build the list and sort it at the end,
or sort it not at all.
Add path_list_append() and sort_path_list() to allow that.
Also, add the unsorted_path_list_has_path() function, to do a linear
search.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
I should have done that long ago.
path-list.c | 30 ++++++++++++++++++++++++++++++
path-list.h | 8 +++++++-
2 files changed, 37 insertions(+), 1 deletions(-)
diff --git a/path-list.c b/path-list.c
index 3d83b7b..92e5cf2 100644
--- a/path-list.c
+++ b/path-list.c
@@ -102,3 +102,33 @@ void print_path_list(const char *text, const struct path_list *p)
for (i = 0; i < p->nr; i++)
printf("%s:%p\n", p->items[i].path, p->items[i].util);
}
+
+struct path_list_item *path_list_append(const char *path, struct path_list *list)
+{
+ ALLOC_GROW(list->items, list->nr + 1, list->alloc);
+ list->items[list->nr].path =
+ list->strdup_paths ? xstrdup(path) : (char *)path;
+ return list->items + list->nr++;
+}
+
+static int cmp_items(const void *a, const void *b)
+{
+ const struct path_list_item *one = a;
+ const struct path_list_item *two = b;
+ return strcmp(one->path, two->path);
+}
+
+void sort_path_list(struct path_list *list)
+{
+ qsort(list->items, list->nr, sizeof(*list->items), cmp_items);
+}
+
+int unsorted_path_list_has_path(struct path_list *list, const char *path)
+{
+ int i;
+ for (i = 0; i < list->nr; i++)
+ if (!strcmp(path, list->items[i].path))
+ return 1;
+ return 0;
+}
+
diff --git a/path-list.h b/path-list.h
index 5931e2c..ca2cbba 100644
--- a/path-list.h
+++ b/path-list.h
@@ -13,10 +13,16 @@ struct path_list
};
void print_path_list(const char *text, const struct path_list *p);
+void path_list_clear(struct path_list *list, int free_util);
+/* Use these functions only on sorted lists: */
int path_list_has_path(const struct path_list *list, const char *path);
-void path_list_clear(struct path_list *list, int free_util);
struct path_list_item *path_list_insert(const char *path, struct path_list *list);
struct path_list_item *path_list_lookup(const char *path, struct path_list *list);
+/* Use these functions only on unsorted lists: */
+struct path_list_item *path_list_append(const char *path, struct path_list *list);
+void sort_path_list(struct path_list *list);
+int unsorted_path_list_has_path(struct path_list *list, const char *path);
+
#endif /* PATH_LIST_H */
--
1.5.3.7.2157.g9598e
^ permalink raw reply related
* Re: [PATCH] git-checkout --push/--pop
From: David Kågedal @ 2007-12-05 17:44 UTC (permalink / raw)
To: git
In-Reply-To: <vpqir3de8t6.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Matthieu, is this something that forgetful people would find useful?
>
> Not sure. That's obviously an interesting feature, but adding two more
> options to checkout (which is already a huge swiss-army knife) might
> not be worth the trouble.
>
> And the issue with push/pop approaches is that I usually notice I have
> to use pop after not having used push (i.e. I use "cd -" all the time,
> but rarely "pushd"/"popd").
It is probably more common that you want to be able to switch back to
the previous branch, than that you actually need a full stack.
So a "git checkout --previous" could be enough. Or a set of aliases
[alias]
co = !"git symbolic-ref HEAD | sed -ne 's!refs/heads/!!p' > .git/LAST ; git checkout"
pop = !"git co $(cat .git/LAST)"
--
David Kågedal
^ permalink raw reply
* Re: fetch_refs_via_pack() discards status?
From: Daniel Barkalow @ 2007-12-05 19:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, Git Mailing List
In-Reply-To: <7vk5nt1v7k.fsf@gitster.siamese.dyndns.org>
On Tue, 4 Dec 2007, Junio C Hamano wrote:
> The code calls fetch_pack() to get the list of refs it fetched, and
> discards refs and always returns 0 to signal success.
>
> But builtin-fetch-pack.c::fetch_pack() has error cases. The function
> returns NULL if error is detected (shallow-support side seems to choose
> to die but I suspect that is easily fixable to error out as well).
>
> Shouldn't fetch_refs_via_pack() propagate that error to the caller?
I think that's right. I think I got as far as having the error status from
fetch_pack() actually returned correctly, and then failed to look at it.
I'd personally avoid testing a pointer to freed memory, but that's
obviously not actually wrong.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH] Add option --path to allow to run tests with real systems
From: Junio C Hamano @ 2007-12-05 19:32 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <20071205134522.GA24617@laptop>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 90b6844..50a3551 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -84,6 +84,8 @@ do
> --no-python)
> # noop now...
> shift ;;
> + --path=*)
> + path="${1#*=}"; shift ;;
> *)
> break ;;
> esac
> @@ -296,11 +298,19 @@ test_done () {
>
> # Test the binaries we have just built. The tests are kept in
> # t/ subdirectory and are run in trash subdirectory.
> -PATH=$(pwd)/..:$PATH
> -GIT_EXEC_PATH=$(pwd)/..
> +if [ -n "$path" ]; then
> + [ -x "$path/git" ] || error "git not found in $path"
> + PATH="$path":$PATH
> + export PATH
> + GIT_EXEC_PATH="$(git --exec-path)"
This is wrong, isn't it? $path may point at the freshly built but not
installed git executable, but it reports --exec-path the location that
git-foo and friends are to be _eventually_ installed, not the location
they are sitting after built, being tested, waiting to be installed.
^ permalink raw reply
* Re: [PATCH] git config: Don't rely on regexec() returning 1 on non-match
From: Junio C Hamano @ 2007-12-05 19:34 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Björn Steinbrink, gitster, git
In-Reply-To: <Pine.LNX.4.64.0712051513570.27959@racer.site>
Thanks. I'll apply but http://git.pastebin.com/m24a4e277 feels somewhat
safer and less intrusive.
^ permalink raw reply
* Re: [PATCH 6/6] DWIM "git add remote ..."
From: Linus Torvalds @ 2007-12-05 19:34 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <Pine.LNX.4.64.0712051902510.27959@racer.site>
On Wed, 5 Dec 2007, Johannes Schindelin wrote:
>
> It is wrong to divert to "git remote add" when you typed the
> (more English) "git add remote". But is it also pretty convenient.
Please don't do cute things like this. Suddenly "git add remote" has two
different meanings depending on whether you have a file called "remote" or
not. That is *not* ok.
Not to mention the fact that your patch is also a horrible piece of
bug-infested shit, to put it less-than-politely. Namely, you now broke
"git add remote" when "remote" is a symbolic link pointing to hyperspace.
But even if you fix that bug, it only goes to show just how misguided
things like these are. It's a *lot* more important to make sense and not
have surprising special cases, than to try to make git command lines act
as if they were free-form English.
Linus
^ permalink raw reply
* Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV
From: Junio C Hamano @ 2007-12-05 19:38 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Ramsay Jones, Arjen Laarhoven, Brian Gernhardt
In-Reply-To: <1196869526-2197-1-git-send-email-jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> diff --git a/configure.ac b/configure.ac
> index 5f8a15b..5d2936e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -212,6 +212,27 @@ test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket"
>
>
> ## Checks for header files.
> +AC_MSG_NOTICE([CHECKS for header files])
> +#
> +# Define OLD_ICONV if your library has an old iconv(), where the second
> +# (input buffer pointer) parameter is declared with type (const char **).
> +AC_DEFUN([OLDICONVTEST_SRC], [[
> +#include <iconv.h>
> +
> +int main(void)
> +{
> + iconv_t cd;
> + char *ibp, *obp;
> + size_t insz, outsz;
> + iconv(cd, &ibp, &insz, &obp, &outsz);
> +}
> +]])
> +AC_MSG_CHECKING([for old iconv()])
> +AC_COMPILE_IFELSE(OLDICONVTEST_SRC,
> + [AC_MSG_RESULT([no])],
> + [AC_MSG_RESULT([yes])
> + OLD_ICONV=YesPlease])
> +AC_SUBST(OLD_ICONV)
>
Which result does COMPILE_IFELSE give for non error warnings? Ok, or
Bad?
^ permalink raw reply
* Re: [PATCH] Soft aliases: add "less" and minimal documentation
From: Junio C Hamano @ 2007-12-05 19:45 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0712051131120.27959@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Now you can use "git less HEAD" to view the raw HEAD commit object. It
> is really a soft alias (i.e. it can be overridden by any user-specified
> alias) to "-p cat-file -p".
I actually regret to have suggested "git less". Not only because you
can always say "git show" instead, but because the error message you
would get with usage string will _not_ say "git-less", but some other
command's name if you say "git less nonsense".
I on the other hand find the "view" alias moderately less problematic.
As long as the future direction for the "view" alias is to allow it to
notice user preference and launch something other than the default
"gitk", iow, it is crystal clear that "git view" is just a short-hand
for launching a history browser and the users are free to choose
whichever viewer available, it won't feel inconsistent if underlying
"gitk" barfed on malformed input using its own name.
But then the users can do all that themselves. People who like qgit do
not have to configure "git view" to launch qgit but instead run their
favorite program directly. One thing the built-in alias is possibly
bringing to the table is to give smaller number of commands people need
to learn, without having to know "gitk", "qgit", "tig", "gitview",
"instaweb", and possibly others, while at the same time enforcing a
policy that the history viewer of choice is aliased to "git view" (not
"git viewer" or "git visualize") to maintain a bit of consistency across
users.
By extension to this reasoning, I am not too keen on adding "update",
"up", "checkin", "ci", nor "co". I do not think of any alternative
backend implementations to these aliases, which means that there isn't
even the advantage of giving a single front-end that lets the user do
the same thing using a choice from multiple backends and keeps the
interface simple for these names.
^ permalink raw reply
* Re: [PATCH] gitweb: Try harder in parse_tag; perhaps it was given ambiguous name
From: Junio C Hamano @ 2007-12-05 19:46 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Guillaume Seguin
In-Reply-To: <200712051113.40654.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> First patch, which is modified version of Guillaume Seguin patch solves
> problem that links in gitweb does lead to correct 'tag' view, while the
> second one solves the problem from the other side: instead of ensuring
> that links in gitweb are unambiguous it tries to resolve ambiguity.
Ok, I'll queue the first one (disambiguate) for 1.5.4 while letting the
people decide the latter for now.
> The problem is caused by the fact that git _always_ prefer heads (head
> refs) to tags (tag refs), even when it is clear
> $ git cat-file tags ambiguous-ref
> that we want a tag. So alternate solution would be to correct
> git-cat-file.
You are getting the layering all wrong.
* git-cat-file takes "object name" on its command line (so do many
other commands).
* One of the way to spell an "object name" is to refer to it with a ref
that can reach it (e.g. to name 12th generation parent of the tip of
the master branch, you spell "master~12" and you are using the ref
refs/heads/master).
* You do not have to always write out the ref in full. There is a
defined order to disambiguate refs (see git-rev-parse(1)), that
allows you to say 'master' and it expands to either refs/tags/master,
refs/heads/master or whatever.
Now git-cat-file does not care how you spelled your object name, and has
no business influencing the ref disambiguation order. You _could_ argue
"git cat-file tag <foo>" _expects_ <foo> to name a tag, but that logic
is very flawed (and that is why I said your understanding of layering is
screwed) for two reasons:
(1) <foo> may be user input to the script that uses cat-file and the
script may be expecting a tag there. Perhaps the script is about
creating a new branch from a tag (expecting a tag) and adds some
administrative info in the configuration file for the branch. It
does first:
t=$(git cat-file tag "$1") || die not a tag
and later the script may want to do:
git branch $newone "$1"
git config branch.$newone.description "created from tag $1 ($t)"
If you make "cat-file tag" to favor tag, and in a similar fashion
if you make "branch" favor branch, the above will not do what you
expect.
Consistently resolving the refname without (or minimum number of)
exceptions would give less surprising result.
(2) "git cat-file -t <foo>" is to find out what type the object is and
is meant to be used by callers who do not know the type. There is
no "favoring this class of ref over other classses" possible there.
> It would be quite easy I think to add checking if gitweb returns
> expected HTTP return code (HTTP status). So what is the portable way
> to check if first line of some output matches given regexp (given fixed
> string)?
Huh? Wouldn't something like this be enough?
>expect.empty &&
cmd >actual.out 2>actual.err &&
diff -u expect.empty actual.err &&
first=$(sed -e '1q' <actual.out) &&
test "z$first" = "I like it"
^ permalink raw reply
* Re: [PATCH] Color support for "git-add -i"
From: Junio C Hamano @ 2007-12-05 19:46 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Dan Zwell, Jeff King, Wincent Colaiuta, git
In-Reply-To: <475697BC.2090701@viscovery.net>
Johannes Sixt <j.sixt@viscovery.net> writes:
> Junio C Hamano schrieb:
>> +color.interactive::
>> + When true (or `always`), always use colors in `git add
>> + --interactive`. When false (or `never`), never. When set to
>> + `auto`, use colors only when the output is to the
>> + terminal. Defaults to false.
>
> Any particular reason why color.interactive = true should be different from
> color.diff = true? See 57f2b842 ("color.diff = true" is not "always" anymore)
Leftover from the original series. Thanks for noticing.
We will probably want that "git config --colorbool" thing we discussed
earlier.
^ permalink raw reply
* Re: [PATCH 6/6] DWIM "git add remote ..."
From: Johannes Schindelin @ 2007-12-05 19:47 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git, gitster
In-Reply-To: <alpine.LFD.0.9999.0712051129280.13796@woody.linux-foundation.org>
Hi,
On Wed, 5 Dec 2007, Linus Torvalds wrote:
> On Wed, 5 Dec 2007, Johannes Schindelin wrote:
> >
> > It is wrong to divert to "git remote add" when you typed the (more
> > English) "git add remote". But is it also pretty convenient.
>
> Please don't do cute things like this. Suddenly "git add remote" has two
> different meanings depending on whether you have a file called "remote"
> or not. That is *not* ok.
>
> Not to mention the fact that your patch is also a horrible piece of
> bug-infested shit, to put it less-than-politely. Namely, you now broke
> "git add remote" when "remote" is a symbolic link pointing to
> hyperspace.
>
> But even if you fix that bug, it only goes to show just how misguided
> things like these are. It's a *lot* more important to make sense and not
> have surprising special cases, than to try to make git command lines act
> as if they were free-form English.
Hehe. You're right, of course. But I could not resist showing off that
patch which I wrote after I mixed up the command line of "remote add" for
the 926th time.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV
From: Wincent Colaiuta @ 2007-12-05 20:49 UTC (permalink / raw)
To: Jakub Narebski
Cc: git, Junio C Hamano, Ramsay Jones, Arjen Laarhoven,
Brian Gernhardt
In-Reply-To: <200712051752.28667.jnareb@gmail.com>
El 5/12/2007, a las 17:52, Jakub Narebski escribió:
> On Wed, 5 December 2007, Wincent Colaiuta wrote:
>>
>> Before applying your patch:
>>
>> CC utf8.o
>> utf8.c: In function ‘reencode_string’:
>> utf8.c:328: warning: passing argument 2 of ‘iconv’ from incompatible
>> pointer type
>> CC convert.o
>>
>> After applying your patch:
>>
>> CC utf8.o
>> CC convert.o
>
> Do I understand correctly that above is excerpt from the output of the
> following sequence of commands before and after this patch applied?
>
> $ make configure
> $ ./configure [options]
> $ make
Yes, that's right, but with a "make clean" before anything else.
> Do you have something like below in ./configure output?
>
> configure: CHECKS for header files
> checking for old iconv()... yes
This:
configure: CHECKS for header files
checking for old iconv()... no
>> This on Darwin Kernel Version 9.1.0 (Mac OS X 10.5.1).
>
> Strange... in Makefile there is
>
> ifeq ($(uname_S),Darwin)
> NEEDS_SSL_WITH_CRYPTO = YesPlease
> NEEDS_LIBICONV = YesPlease
> OLD_ICONV = UnfortunatelyYes
> NO_STRLCPY = YesPlease
> NO_MEMMEM = YesPlease
> endif
>
> so the uname based guessing should set OLD_ICONV on Darwin...
That happens *before* config.mak.autogen is included in the Makefile,
so it gets overridden.
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH] gitweb: Try harder in parse_tag; perhaps it was given ambiguous name
From: Jakub Narebski @ 2007-12-05 21:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Guillaume Seguin
In-Reply-To: <7v7ijsq5zm.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> > It would be quite easy I think to add checking if gitweb returns
> > expected HTTP return code (HTTP status). So what is the portable way
> > to check if first line of some output matches given regexp (given fixed
> > string)?
>
> Huh? Wouldn't something like this be enough?
>
> >expect.empty &&
> cmd >actual.out 2>actual.err &&
> diff -u expect.empty actual.err &&
> first=$(sed -e '1q' <actual.out) &&
> test "z$first" = "I like it"
Well, actually that is even better idea. We can go for one of the three
levels of HTTP status checking:
1. Check if we got "Status: 200 OK" when we expect it, and not have it
when we expect other HTTP status, e.g. when requesting nonexistent
file. The above code is enough for that.
2. We can check if we got expected status number, for example 200 for
when we expect no error, or 404 when object is not found, or 403
if there is no such object etc. I was thinking about using this version
the need to check not full first line, but fragment of it.
3. We can check full first line, for example
Status: 200 OK
Status: 403 Forbidden
Status: 404 Not Found
Status: 400 Bad Request
but this might tie gitweb test too tightly with minute details of
gitweb output. The above code is good for that too.
What do you think, which route we should go in test?
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: fetch_refs_via_pack() discards status?
From: Junio C Hamano @ 2007-12-05 21:06 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Shawn O. Pearce, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0712051356040.5349@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
> On Tue, 4 Dec 2007, Junio C Hamano wrote:
>
>> The code calls fetch_pack() to get the list of refs it fetched, and
>> discards refs and always returns 0 to signal success.
>>
>> But builtin-fetch-pack.c::fetch_pack() has error cases. The function
>> returns NULL if error is detected (shallow-support side seems to choose
>> to die but I suspect that is easily fixable to error out as well).
>>
>> Shouldn't fetch_refs_via_pack() propagate that error to the caller?
>
> I think that's right. I think I got as far as having the error status from
> fetch_pack() actually returned correctly, and then failed to look at it.
> I'd personally avoid testing a pointer to freed memory, but that's
> obviously not actually wrong.
>
> -Daniel
Hmph, is that an Ack that the patchlet is actually a bugfix?
^ permalink raw reply
* Re: [PATCH] gitweb: Try harder in parse_tag; perhaps it was given ambiguous name
From: Junio C Hamano @ 2007-12-05 21:14 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Guillaume Seguin
In-Reply-To: <200712052202.27111.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> Well, actually that is even better idea. We can go for one of the three
> levels of HTTP status checking:
>
> 1. Check if we got "Status: 200 OK" when we expect it, and not have it
> when we expect other HTTP status, e.g. when requesting nonexistent
> file. The above code is enough for that.
>
> 2. We can check if we got expected status number, for example 200 for
> when we expect no error, or 404 when object is not found, or 403
> if there is no such object etc. I was thinking about using this version
> the need to check not full first line, but fragment of it.
>
> 3. We can check full first line, for example
> Status: 200 OK
> Status: 403 Forbidden
> Status: 404 Not Found
> Status: 400 Bad Request
> but this might tie gitweb test too tightly with minute details of
> gitweb output. The above code is good for that too.
>
> What do you think, which route we should go in test?
4. We should check for what we expect in the parts of gitweb output we
care about. E.g.
* If we are making sure it refuses to serve incorrect request
(e.g. no such repository), we should check for the status, which is
what we care about (we may not care about how the actual error
message is stated).
* Otherwise (and I suspect this is "most of the tests"), we obviously
expect to have "200 OK", and check for that;
BUT IN ADDITION
* If we want to make sure that a specific aspect of the output was
buggy and a patch fixed it (e.g. an href used a short refname
without having refs/heads or refs/tags prefixed, causing
ambiguity), we also should check that part of output in generated
HTML, not just the HTTP status header.
^ permalink raw reply
* Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV
From: Jakub Narebski @ 2007-12-05 21:19 UTC (permalink / raw)
To: Wincent Colaiuta
Cc: git, Junio C Hamano, Ramsay Jones, Arjen Laarhoven,
Brian Gernhardt
In-Reply-To: <52A4CC8B-EB11-4E3F-A3B6-06826F860E5D@wincent.com>
Wincent Colaiuta wrote:
> El 5/12/2007, a las 17:52, Jakub Narebski escribió:
>> On Wed, 5 December 2007, Wincent Colaiuta wrote:
>>>
>>> Before applying your patch:
>>>
>>> CC utf8.o
>>> utf8.c: In function ‘reencode_string’:
>>> utf8.c:328: warning: passing argument 2 of ‘iconv’ from incompatible
>>> pointer type
>>> CC convert.o
>>>
>>> After applying your patch:
>>>
>>> CC utf8.o
>>> CC convert.o
>>
>> Do I understand correctly that above is excerpt from the output of the
>> following sequence of commands before and after this patch applied?
>>
>> $ make configure
>> $ ./configure [options]
>> $ make
>
> Yes, that's right, but with a "make clean" before anything else.
>
>> Do you have something like below in ./configure output?
>>
>> configure: CHECKS for header files
>> checking for old iconv()... yes
>
> This:
>
> configure: CHECKS for header files
> checking for old iconv()... no
>
>>> This on Darwin Kernel Version 9.1.0 (Mac OS X 10.5.1).
>>
>> Strange... in Makefile there is
>>
>> ifeq ($(uname_S),Darwin)
>> NEEDS_SSL_WITH_CRYPTO = YesPlease
>> NEEDS_LIBICONV = YesPlease
>> OLD_ICONV = UnfortunatelyYes
>> NO_STRLCPY = YesPlease
>> NO_MEMMEM = YesPlease
>> endif
>>
>> so the uname based guessing should set OLD_ICONV on Darwin...
>
> That happens *before* config.mak.autogen is included in the Makefile,
> so it gets overridden.
Ahhh... now I understand. You have installed new iconv() on your
computer, and generic 'uname -s' (OS name) based guessing in Makefile
guesses wrongly that you need OLD_ICONV, while ./configure script
actually tests it and correctly decides to unset OLD_ICONV !
BTW. Perhaps it whould be written more explicitely:
+AC_COMPILE_IFELSE(OLDICONVTEST_SRC,
+ [AC_MSG_RESULT([no])
+ OLD_ICONV=],
+ [AC_MSG_RESULT([yes])
+ OLD_ICONV=YesPlease])
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV
From: Jakub Narebski @ 2007-12-05 21:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Ramsay Jones, Arjen Laarhoven, Brian Gernhardt
In-Reply-To: <7vtzmxortn.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
>> +AC_MSG_CHECKING([for old iconv()])
>> +AC_COMPILE_IFELSE(OLDICONVTEST_SRC,
>> + [AC_MSG_RESULT([no])],
>> + [AC_MSG_RESULT([yes])
>> + OLD_ICONV=YesPlease])
>> +AC_SUBST(OLD_ICONV)
>>
>
> Which result does COMPILE_IFELSE give for non error warnings? Ok, or
> Bad?
- Macro: AC_COMPILE_IFELSE (INPUT, [ACTION-IF-FOUND],
[ACTION-IF-NOT-FOUND])
Run the compiler and compilation flags of the current language
(*note Language Choice::) on the INPUT, run the shell commands
ACTION-IF-TRUE on success, ACTION-IF-FALSE otherwise. The INPUT
can be made by `AC_LANG_PROGRAM' and friends.
And if I have checked correctly code which causes only warnings
returns Ok (in this case print 'no' after 'checking for old iconv()... '
and do not set OLD_ICONV, which means it will be unset).
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: git-svn: .git/svn disk usage
From: Steven Grimm @ 2007-12-05 21:30 UTC (permalink / raw)
To: Eric Wong; +Cc: David Voit, git
In-Reply-To: <20071205085451.GA347@soma>
How about using git itself to keep some of this information? I'll just
throw this idea out there; might or might not make any actual sense.
Create a new "git-svn metadata" branch. This branch contains a fake
directory (never intended for checkout, though you could do it) that
has a "file" for each svn revision. The filename is just the svn
revision number, maybe divided into subdirectories in case you want to
check the branch out for debugging purposes or whatever. The contents
are the git commit SHA1 and whatever other metadata you want to keep
in the future.
The advantage of doing it this way? You can pass around svn metadata
using the normal git fetch/push tools, query the metadata using "git
show", etc. In terms of data integrity, it's as secure as anything
else in a git repository, much more so than a separately maintained db
file under .git.
Along similar lines, a separate branch where the filenames are commit
SHA1s and the file contents are the stuff that currently gets written
into the git-svn-id: lines would mean no more need to rewrite history
when doing dcommit, and thus easier mixing of native git workflows and
interactions with an svn repository.
It would be great if you could clone a git-svn repository and then do
"git svn dcommit" from the clone, secure in the knowledge that things
will stay consistent even if the origin gets your changes via "git svn
fetch" rather than from you.
-Steve
^ permalink raw reply
* Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV
From: Brian Gernhardt @ 2007-12-05 21:38 UTC (permalink / raw)
To: Jakub Narebski
Cc: Wincent Colaiuta, git, Junio C Hamano, Ramsay Jones,
Arjen Laarhoven
In-Reply-To: <200712052219.00930.jnareb@gmail.com>
On Dec 5, 2007, at 4:19 PM, Jakub Narebski wrote:
> Ahhh... now I understand. You have installed new iconv() on your
> computer, and generic 'uname -s' (OS name) based guessing in Makefile
> guesses wrongly that you need OLD_ICONV, while ./configure script
> actually tests it and correctly decides to unset OLD_ICONV !
As far as the "installed new iconv()" goes, you may be wrong there.
The latest major release of OS X includes a version of iconv that does
not need OLD_ICONV as part of the base system. I've had to unset it
in my config.mak in order to avoid the warning.
~~ Brian Gernhardt
^ permalink raw reply
* [PATCH 7/6] builtin-remote: guard remote->url accesses by remote->url_nr check
From: Johannes Schindelin @ 2007-12-05 21:40 UTC (permalink / raw)
To: git, gitster; +Cc: Johannes Sixt
In-Reply-To: <Pine.LNX.4.64.0712051902080.27959@racer.site>
struct remote's url member is not a NULL terminated list. Instead, we
have to check url_nr before accessing it.
Noticed by Johannes Sixt.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
For your viewing pleasure, this is not a resend, but an amend.
Feel free to squash with 4/6, or to bug me to do it.
builtin-remote.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/builtin-remote.c b/builtin-remote.c
index 142eb97..ea323e2 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -356,7 +356,8 @@ static int show_or_prune(int argc, const char **argv, int prune)
states.remote = remote_get(*argv);
if (!states.remote)
return error("No such remote: %s", *argv);
- transport = transport_get(NULL, states.remote->url[0]);
+ transport = transport_get(NULL, states.remote->url_nr > 0 ?
+ states.remote->url[0] : NULL);
ref = transport_get_remote_refs(transport);
read_branches();
@@ -391,7 +392,7 @@ static int show_or_prune(int argc, const char **argv, int prune)
}
printf("* remote %s\n URL: %s\n", *argv,
- states.remote->url[0] ?
+ states.remote->url_nr > 0 ?
states.remote->url[0] : "(no URL)");
for (i = 0; i < branch_list.nr; i++) {
@@ -468,8 +469,9 @@ static int get_one_entry(struct remote *remote, void *priv)
{
struct path_list *list = priv;
- path_list_append(remote->name, list)->util = (void *)remote->url[0];
- if (remote->url[0] && remote->url[1])
+ path_list_append(remote->name, list)->util = remote->url_nr ?
+ (void *)remote->url[0] : NULL;
+ if (remote->url_nr > 1)
warning ("Remote %s has more than one URL", remote->name);
return 0;
--
1.5.3.7.2157.g9598e
^ permalink raw reply related
* Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV
From: Pascal Obry @ 2007-12-05 21:46 UTC (permalink / raw)
To: Jakub Narebski
Cc: git, Junio C Hamano, Ramsay Jones, Arjen Laarhoven,
Brian Gernhardt
In-Reply-To: <1196869526-2197-1-git-send-email-jnareb@gmail.com>
Jakub Narebski a écrit :
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> This patch needs checking if it correctly sets OLD_ICONV
> when needed. I have checked only that it is not set when
> with new iconv() declaration. Could people using Cygwin
> (and other with OLD_ICONV: Darwin) test it?
Not working on Cygwin:
$ autoconf
$ ./configure --prefix=/usr/local --build=i686-pc-cygwin
...
configure: CHECKS for header files
checking for old iconv()... no
It should be yes above. And in config.mak.autogen we have:
OLD_ICONV=
Note also that you should remove all the hard-coded settings in Makefile
anyway.
Pascal.
--
--|------------------------------------------------------
--| Pascal Obry Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--| http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595
^ permalink raw reply
* Re: What's cooking in git.git (topics)
From: Miles Bader @ 2007-12-05 21:58 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: David Kastrup, Jakub Narebski, git
In-Reply-To: <alpine.LFD.0.99999.0711261601240.9605@xanadu.home>
Nicolas Pitre <nico@cam.org> writes:
> Yet Junio just replied to my mail, apparently using his news reader, and
> I was directly addressed.
If you're using Gnus as a MUA, and reading via gmane, you can bypass
gmane for followups by setting the group "To List" parameter to the list
address ("git@vger.kernel.org"); be careful _not_ to set the adjacent
"To Address" parameter, which does something else.
After doing that, followups are sent via email with To and CCs correctly
set up, exactly as if you were reading an ordinary mailing list.
I presume other newsreaders having something similar.
-Miles
[You can hit C-M-a in the group summary buffer to modify group parameters]
--
`Life is a boundless sea of bitterness'
^ permalink raw reply
* Re: [PATCH 3/6] Test "git remote show" and "git remote prune"
From: René Scharfe @ 2007-12-05 21:59 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <Pine.LNX.4.64.0712051901550.27959@racer.site>
Johannes Schindelin schrieb:
> While at it, also fix a few instances where a cd was done outside of a
> subshell.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> t/t5505-remote.sh | 34 ++++++++++++++++++++++++++++++++++
> 1 files changed, 34 insertions(+), 0 deletions(-)
It seems to me the patch only adds tests, but doesn't fix existing ones.
And looking at t5505-remote.sh, every call of cd is already done inside
of a subshell, so there doesn't seem to be anything to fix either. :-?
^ permalink raw reply
* Re: builtin command's prefix question
From: Junio C Hamano @ 2007-12-05 22:12 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List
In-Reply-To: <fcaeb9bf0712050856t5d730779q82783fdb9876f41@mail.gmail.com>
"Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:
> I have been looking at setup_git_directory_gently() lately. From my
> understanding, setup_git_directory* will return a string called
> "prefix" that is passed to builtin commands. What is the exact meaning
> of this prefix?
Some historical background is in order. For a long time, we only worked
from the very top of the work tree and nowhere else. The path you get
from the user on the command line was supposed to be relative to the top
of the work tree, the user was supposed to be at the top of the work
tree, no work from subdirectories.
This was limiting, so "setup" was introduced. The commands originally
worked only from the top would chdir up to the top of the work tree if
it was run from a subdirectory. This however needs adjustments to the
paths we get from the user from the command line (or stdin for commands
that take such). If we claim we work from a subdirectory, we should
interpret path given by the user who is in a subdirectory as relative to
that subdirectory. The way to do this adjustment is to prefix the
subdirectory path to the path given by the user.
So, if you start a command from Documentation/ subdirectory, like this:
$ cd Documentation
$ git ls-files howto
internally, ls-files would:
* Notice it was run from Documentation/ subdirectory;
* cd up to the top level;
* prefix "Documentation/" to the pathspec given by the user
(i.e. "howto"), to form "Documentation/howto";
* Act as before, except that it strips "Documentation/" from its usual
output, to retain the illusion of working from that subdirectory.
And prefix is "Documentation/" (note the trailing slash) in such a
case. If you run from the top, it is NULL to signal that there is no
need to do any of these tricks.
> ... Correct me if I'm wrong. In early (read: no worktree)
> days, cwd was moved to working root directory and prefix contained
> relative path from working root directory to the original cwd. So it
> had a few implications:
> 1. A non-empty prefix indicates cwd has been moved
Correct (I do not know if we care, though)
> 2. If cwd is moved, it is moved to working root directory
Correct ("we always work from top of the tree internally" matters)
> 3. cwd+prefix should point to the current directory at the time the
> command was issued (let's call it "user cwd")
Correct.
> Things change a bit since the rise of worktree:
> - If GIT_DIR is set and GIT_WORK_TREE is not, prefix is relative to
> the to-be-set-up worktree, but cwd is not changed, so point 3 is gone.
> - If GIT_DIR is not set and GiT_WORK_TREE is,
> - and it is found that user cwd is inside a gitdir (bare repo), cwd
> has been moved and prefix is empty, cwd+prefix no longer point to user
> cwd
> - for other cases, cwd may not be worktree (the real worktree will
> be setup in setup_work_tree or setup_git_directory)
The intention is:
* If GIT_DIR is set but not GIT_WORK_TREE (nor core.worktree in
config), you are either inside project.git/ directory of bare
repository or at the toplevel of worktree-full directory. This has
been the traditional behaviour before GIT_WORK_TREE and we shouldn't
break the existing setups that assume this behaviour. So in that
sense, with this combination:
- If the repository is bare, the value of the prefix should not
matter; the command that wants to look at prefix by definition
wants to run from a subdirectory but there is no notion of
"the user directory being a subdirectory of the top of the work
tree" in a bare repository;
- If the repository is not bare, the user directory _MUST_ be at the
top of the work tree, as that is what the traditional behaviour is.
Anything else would break existing setups.
IOW, if you use GIT_DIR and still want to run from a subdirectory
of the worktree, you must have either GIT_WORK_TREE or
core.worktree to tell where the top of the worktree is, and if you
don't, then you must be at the top.
So the right thing to do in this case is not going anywhere and using
prefix=NULL.
* I would say it is a misconfiguration if GIT_DIR is not set and
GIT_WORK_TREE is, as the sole purpose of GIT_WORK_TREE is so that you
can work from a subdirectory when you set GIT_DIR. I may be missing
an obvious use case that this is useful, but I do not think of any.
Dscho may be able to correct me on this, as he fixed up the original
work tree series that was even messier quite a bit during the last
round.
^ 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