* Re: [PATCH] Support tags in uncommit - use git_id instead of rev_parse
From: Pavel Roskin @ 2007-10-03 21:44 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0710031335o1c7f3a10i6f2055b76376bfd4@mail.gmail.com>
On Wed, 2007-10-03 at 21:35 +0100, Catalin Marinas wrote:
> Without this patch, the 'stg uncommit -t patch' fails with 'Unknown
> revision: patch'. With the patch applied, it still fails but with
> 'Commit ... does not have exactly one parent'. I don't say that the
> first one is good but I don't think the latter is clearer. The 'stg
> uncommit --help' states that the '--to' option takes a commit argument
> but if one passes a patch name the error message gets pretty
> confusing.
Actually, 'Commit ... does not have exactly one parent' means that stg
misinterpreted the patch name as some non-existing hash and started
iterating back until it hit the first merge.
Perhaps stgit should make sure that the hash is valid before walking the
commit tree. If it's not, stgit could provide a better message.
--
Regards,
Pavel Roskin
^ permalink raw reply
* [PATCH] Add a simple option parser.
From: Kristian Høgsberg @ 2007-10-03 21:45 UTC (permalink / raw)
To: git; +Cc: gitster, Kristian Høgsberg
The option parser takes argc, argv, an array of struct option
and a usage string. Each of the struct option elements in the array
describes a valid option, its type and a pointer to the location where the
value is written. The entry point is parse_options(), which scans through
the given argv, and matches each option there against the list of valid
options. During the scan, argv is rewritten to only contain the
non-option command line arguments and the number of these is returned.
Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
Makefile | 2 +-
parse-options.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
parse-options.h | 33 +++++++++++++++++
3 files changed, 140 insertions(+), 1 deletions(-)
create mode 100644 parse-options.c
create mode 100644 parse-options.h
diff --git a/Makefile b/Makefile
index 62bdac6..d90e959 100644
--- a/Makefile
+++ b/Makefile
@@ -310,7 +310,7 @@ LIB_OBJS = \
alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
- transport.o bundle.o
+ transport.o bundle.o parse-options.o
BUILTIN_OBJS = \
builtin-add.o \
diff --git a/parse-options.c b/parse-options.c
new file mode 100644
index 0000000..130b609
--- /dev/null
+++ b/parse-options.c
@@ -0,0 +1,106 @@
+#include "git-compat-util.h"
+#include "parse-options.h"
+
+static int parse_one(const char **argv,
+ struct option *options, int count,
+ const char *usage_string)
+{
+ const char *eq, *arg, *value;
+ int i, processed;
+
+ arg = argv[0];
+ value = NULL;
+
+ if (arg[0] != '-')
+ return 0;
+
+ for (i = 0; i < count; i++) {
+ if (arg[1] == '-') {
+ if (!prefixcmp(options[i].long_name, arg + 2)) {
+ if (options[i].type != OPTION_BOOLEAN) {
+ value = argv[1];
+ processed = 2;
+ } else {
+ processed = 1;
+ }
+ break;
+ }
+
+ eq = strchr(arg + 2, '=');
+ if (eq && options[i].type != OPTION_BOOLEAN &&
+ !strncmp(arg + 2,
+ options[i].long_name, eq - arg - 2)) {
+ value = eq + 1;
+ processed = 1;
+ break;
+ }
+ }
+
+ if (arg[1] == options[i].short_name) {
+ if (arg[2] == '\0') {
+ if (options[i].type != OPTION_BOOLEAN) {
+ value = argv[1];
+ processed = 2;
+ } else {
+ processed = 1;
+ }
+ break;
+ }
+
+ if (options[i].type != OPTION_BOOLEAN) {
+ value = arg + 2;
+ processed = 1;
+ break;
+ }
+ }
+ }
+
+ if (i == count)
+ usage(usage_string);
+ else switch (options[i].type) {
+ case OPTION_BOOLEAN:
+ (*(int *)options[i].value)++;
+ break;
+ case OPTION_STRING:
+ if (value == NULL) {
+ error("option %s requires a value.", arg);
+ usage(usage_string);
+ }
+ *(const char **)options[i].value = value;
+ break;
+ case OPTION_INTEGER:
+ if (value == NULL) {
+ error("option %s requires a value.", argv);
+ usage(usage_string);
+ }
+ *(int *)options[i].value = atoi(value);
+ break;
+ default:
+ assert(0);
+ }
+
+ return processed;
+}
+
+int parse_options(int argc, const char **argv,
+ struct option *options, int count,
+ const char *usage_string)
+{
+ int i, j, processed;
+
+ for (i = 1, j = 0; i < argc; ) {
+ if (!strcmp(argv[i], "--"))
+ break;
+ processed = parse_one(argv + i, options, count, usage_string);
+ if (processed == 0)
+ argv[j++] = argv[i++];
+ else
+ i += processed;
+ }
+
+ while (i < argc)
+ argv[j++] = argv[i++];
+ argv[j] = NULL;
+
+ return j;
+}
diff --git a/parse-options.h b/parse-options.h
new file mode 100644
index 0000000..5be9c20
--- /dev/null
+++ b/parse-options.h
@@ -0,0 +1,33 @@
+#ifndef PARSE_OPTIONS_H
+#define PARSE_OPTIONS_H
+
+enum option_type {
+ OPTION_BOOLEAN,
+ OPTION_STRING,
+ OPTION_INTEGER,
+ OPTION_LAST,
+};
+
+struct option {
+ enum option_type type;
+ const char *long_name;
+ char short_name;
+ void *value;
+};
+
+/* Parse the given options against the list of known options. The
+ * order of the option structs matters, in that ambiguous
+ * abbreviations (eg, --in could be short for --include or
+ * --interactive) are matched by the first option that share the
+ * prefix.
+ *
+ * parse_options() will filter out the processed options and leave the
+ * non-option argments in argv[]. The return value is the number of
+ * arguments left in argv[].
+ */
+
+extern int parse_options(int argc, const char **argv,
+ struct option *options, int count,
+ const char *usage_string);
+
+#endif
--
1.5.2.5
^ permalink raw reply related
* Re: [PATCH] Add test case for ls-files --with-head
From: Junio C Hamano @ 2007-10-03 21:47 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Jeff King, Carl Worth, Johannes Sixt, Keith Packard,
Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0710032238080.28395@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> $ for i in {1,2,3,4,5}{0,1,2,3,4,5,6,7,8,9}; do echo $i; done
>> {1,2,3,4,5}{0,1,2,3,4,5,6,7,8,9}
>> $
>
> AFAIK this is the same as bash (I thought I was the last one to make that
> mistake 10 years ago). As long as you do not have _files_ matching the
> pattern, it does not expand. And besides, this is too complicated anyway:
> [1-5] is much shorter than {1,2,3,4,5}.
AFAIK, you are wrong ;-)
{1,2,3,4,5} expands regardless of what's on the filesystem but I
do not think it is POSIX.
[1-5] matches if any of the {1,2,3,4,5} is found on the
filesystem.
^ permalink raw reply
* Re: [PATCH 1/4] Add a simple option parser for use by builtin-commit.c.
From: Kristian Høgsberg @ 2007-10-03 21:53 UTC (permalink / raw)
To: Jonas Fonseca; +Cc: Johannes Schindelin, gitster, git
In-Reply-To: <20071003201129.GB25856@diku.dk>
On Wed, 2007-10-03 at 22:11 +0200, Jonas Fonseca wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote Mon, Oct 01, 2007:
> > On Mon, 1 Oct 2007, Kristian H?gsberg wrote:
> > > On Sun, 2007-09-30 at 15:11 +0200, Jonas Fonseca wrote:
> > > > > +
> > > > > +extern int parse_options(const char ***argv,
> > > > > + struct option *options, int count,
> > > > > + const char *usage_string);
> > > >
> > > > I think the interface could be improved a bit. For example, it doesn't
> > > > need to count argument since the last entry in the options array is
> > > > OPTION_LAST and thus the size can be detected that way.
> > >
> > > Hehe, yeah, that's how I did it first. I don't have a strong preference
> > > for terminator elements vs. ARRAY_SIZE(), but Junio prefers the
> > > ARRAY_SIZE() approach, I guess. At this point I'm just trying the get
> > > the patches upstream...
> >
> > FWIW I like the ARRAY_SIZE() approach better, too, since it is less error
> > prone.
>
> OK, I must have missed that comment. Good point.
>
> Thanks for the comments both of you. It's great to have something to
> work from. However, I also fear it will also require that some extra
> flags or information is added to the option information to make it more
> generally usable. But I guess that is easier to discuss in the context
> of a patch.
I just sent an updated option parser patch that incorporates your
suggestions along with a patch that ports builtin-add.c to use it. I
looked briefly into porting over a few other builtins, but you're right,
we need a couple of extra features for this to be really worthwile:
* OPTION_SET_FLAG: sets the bit (we need to add a bit value that
the option parser can or in)
* OPTION_CLEAR_FLAG: clear the bit
* OPTION_ADD: adds the value to the destination integer
* OPTION_CALLBACK: calls the given function when the option is
matched. We'll need this for builtin-grep that has positional
args such as --and etc.
Also, the option parser should probably verify that a string option
isn't passed more than once. Bundling of single letter options would be
nice to add. But the patch I just sent out should be a good start, and
it lets us move forward with builtin-commit.c.
cheers,
Kristian
^ permalink raw reply
* Re: [PATCH 2/4] This exports the update() function from builtin-add.c as
From: Kristian Høgsberg @ 2007-10-03 22:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7imcv5op.fsf@gitster.siamese.dyndns.org>
On Thu, 2007-09-27 at 02:05 -0700, Junio C Hamano wrote:
...
> I think the right organization for the "builtin-commit" series
> should be:
>
> * merge strbuf topic in kh/commit topic, in order to get the
> stripspace updates and strbuf_read_file();
>
> * add--interactive entry point change (respin the one from the
> old series);
>
> * rename update() to add_files_to_cache() and export (respin
> this [2/4] with a better commit message);
>
> * create a separate rerere() function and export (respin part
> of old series, with proper refactoring);
>
> I am not happy with builtin-foo.c calling into something from
> builtin-bar.c, though. We probably would want to move
> rerere() and add_files_to_cache() somewhere else.
>
> * move launch_editor() and stripspace() to create editor.c (new
> [4/4]);
>
> * add option parser in parse-options.[ch] (new [1/4]);
>
> * finally, create builtin-commit that uses the groundwork laid
> out above (new [3/4]).
>
> I ended up doing the above up to the rerere() one myself, but
> haven't done the rest.
>From what I see in next today, it looks like we're just missing the
parse-options patch and builtin-commit patches. I resent a better
version of parse-options and a patch that ports builtin-add.c to the
option parser. To use the option parser in more places, we'll probably
have to extend it a bit, but the patch is a good start. Let's get that
in shape and into next and then I'll send an updated builtin-commit
patch.
thanks,
Kristian
^ permalink raw reply
* Re: [PATCH] Add test case for ls-files --with-head
From: Jeff King @ 2007-10-03 22:11 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Carl Worth, Johannes Sixt, Keith Packard,
Git Mailing List
In-Reply-To: <7vodffdg6i.fsf@gitster.siamese.dyndns.org>
On Wed, Oct 03, 2007 at 02:47:01PM -0700, Junio C Hamano wrote:
> AFAIK, you are wrong ;-)
>
> {1,2,3,4,5} expands regardless of what's on the filesystem but I
> do not think it is POSIX.
Yes, I think that is right.
-Peff
^ permalink raw reply
* Re: Linking with -R (rpath) not supported on Darwin
From: Brian Gernhardt @ 2007-10-03 22:39 UTC (permalink / raw)
To: Benoit SIGOURE; +Cc: git list
In-Reply-To: <4D954ADB-E66E-43CA-87EE-7522FFA87370@lrde.epita.fr>
On Oct 3, 2007, at 5:34 PM, Benoit SIGOURE wrote:
> Hello,
> I've just compiled HEAD (1.5.3.4.209.g9e417) and saw a:
> LINK git-http-fetch
> i686-apple-darwin8-gcc-4.0.1: unrecognized option '-R/opt/local/lib'
>
> It didn't harm but the build process should be more careful to not
> use options that are not supported by the compiler. And it's not a
> matter of using -Wl,-rpath instead.
I compile git very regularly on my MacBook Pro and have never seen
this error. Do you have the most recent copy of Xcode? I've seen
odd errors on one of the not very old versions of the developer's
tools. For me, `gcc -v` reports "gcc version 4.0.1 (Apple Computer,
Inc. build 5367)".
~~ Brian
^ permalink raw reply
* [PATCH] Be nice with compilers that do not support runtime paths at all.
From: Benoit Sigoure @ 2007-10-03 22:20 UTC (permalink / raw)
To: git; +Cc: Benoit Sigoure
In-Reply-To: <7vsl4rdgf4.fsf@gitster.siamese.dyndns.org>
On Darwin for instance, there is no -R or -Wl,-rpath thing to fiddle with,
it's simply not supported by the dynamic loader. This patch introduces a
NO_RPATH define which is enabled by default for Darwin.
---
Makefile | 24 ++++++++++++++++++++----
1 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index a1fe443..7c6c453 100644
--- a/Makefile
+++ b/Makefile
@@ -100,6 +100,9 @@ all::
# that tells runtime paths to dynamic libraries;
# "-Wl,-rpath=/path/lib" is used instead.
#
+# Define NO_RPATH if your dynamic loader doesn't support runtime paths at
+# all.
+#
# Define USE_NSEC below if you want git to care about sub-second file mtimes
# and ctimes. Note that you need recent glibc (at least 2.2.4) for this, and
# it will BREAK YOUR LOCAL DIFFS! show-diff and anything using it will likely
@@ -507,6 +510,7 @@ ifeq ($(uname_S),Darwin)
BASIC_LDFLAGS += -L/opt/local/lib
endif
endif
+ NO_RPATH = YesPlease
endif
ifdef NO_R_TO_GCC_LINKER
@@ -521,7 +525,10 @@ ifndef NO_CURL
ifdef CURLDIR
# Try "-Wl,-rpath=$(CURLDIR)/$(lib)" in such a case.
BASIC_CFLAGS += -I$(CURLDIR)/include
- CURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$(lib) -lcurl
+ CURL_LIBCURL = -L$(CURLDIR)/$(lib) -lcurl
+ifndef NO_RPATH
+ CURL_LIBCURL += $(CC_LD_DYNPATH)$(CURLDIR)/$(lib)
+endif
else
CURL_LIBCURL = -lcurl
endif
@@ -539,7 +546,10 @@ endif
ifdef ZLIB_PATH
BASIC_CFLAGS += -I$(ZLIB_PATH)/include
- EXTLIBS += -L$(ZLIB_PATH)/$(lib) $(CC_LD_DYNPATH)$(ZLIB_PATH)/$(lib)
+ EXTLIBS += -L$(ZLIB_PATH)/$(lib)
+ifndef NO_RPATH
+ EXTLIBS += $(CC_LD_DYNPATH)$(ZLIB_PATH)/$(lib)
+endif
endif
EXTLIBS += -lz
@@ -547,7 +557,10 @@ ifndef NO_OPENSSL
OPENSSL_LIBSSL = -lssl
ifdef OPENSSLDIR
BASIC_CFLAGS += -I$(OPENSSLDIR)/include
- OPENSSL_LINK = -L$(OPENSSLDIR)/$(lib) $(CC_LD_DYNPATH)$(OPENSSLDIR)/$(lib)
+ OPENSSL_LINK = -L$(OPENSSLDIR)/$(lib)
+ifndef NO_RPATH
+ OPENSSL_LINK = $(CC_LD_DYNPATH)$(OPENSSLDIR)/$(lib)
+endif
else
OPENSSL_LINK =
endif
@@ -564,7 +577,10 @@ endif
ifdef NEEDS_LIBICONV
ifdef ICONVDIR
BASIC_CFLAGS += -I$(ICONVDIR)/include
- ICONV_LINK = -L$(ICONVDIR)/$(lib) $(CC_LD_DYNPATH)$(ICONVDIR)/$(lib)
+ ICONV_LINK = -L$(ICONVDIR)/$(lib)
+ifndef NO_RPATH
+ ICONV_LINK = $(CC_LD_DYNPATH)$(ICONVDIR)/$(lib)
+endif
else
ICONV_LINK =
endif
--
1.5.3.4.209.g9e417
^ permalink raw reply related
* Re: [PATCH] Be nice with compilers that do not support runtime paths at all.
From: Steven Grimm @ 2007-10-03 22:49 UTC (permalink / raw)
To: Benoit Sigoure; +Cc: git
In-Reply-To: <1191450052-23619-1-git-send-email-tsuna@lrde.epita.fr>
Benoit Sigoure wrote:
> On Darwin for instance, there is no -R or -Wl,-rpath thing to fiddle with,
> it's simply not supported by the dynamic loader. This patch introduces a
> NO_RPATH define which is enabled by default for Darwin.
>
I compile git on a MacBook Pro (OS X 10.4, gcc 4.0.1 build 5367 from the
normal Xcode install that comes on the OS install DVD) on a regular
basis. The makefile works fine for me. I suspect there's something else
going on here.
-Steve
^ permalink raw reply
* Re: Linking with -R (rpath) not supported on Darwin
From: Benoit SIGOURE @ 2007-10-03 22:58 UTC (permalink / raw)
To: Brian Gernhardt; +Cc: git list
In-Reply-To: <ECAD7CED-FFA0-46F2-8094-2FDE47CB5D54@silverinsanity.com>
[-- Attachment #1: Type: text/plain, Size: 1368 bytes --]
On Oct 4, 2007, at 12:39 AM, Brian Gernhardt wrote:
> On Oct 3, 2007, at 5:34 PM, Benoit SIGOURE wrote:
>
>> Hello,
>> I've just compiled HEAD (1.5.3.4.209.g9e417) and saw a:
>> LINK git-http-fetch
>> i686-apple-darwin8-gcc-4.0.1: unrecognized option '-R/opt/local/lib'
>>
>> It didn't harm but the build process should be more careful to not
>> use options that are not supported by the compiler. And it's not
>> a matter of using -Wl,-rpath instead.
>
> I compile git very regularly on my MacBook Pro and have never seen
> this error. Do you have the most recent copy of Xcode? I've seen
> odd errors on one of the not very old versions of the developer's
> tools. For me, `gcc -v` reports "gcc version 4.0.1 (Apple
> Computer, Inc. build 5367)".
>
> ~~ Brian
$ gcc -v
[...]
gcc version 4.0.1 (Apple Computer, Inc. build 5367)
I've seen this message for the 1st time when compiling Git after my
nightly git pull today. Anyways, this should be done because there
is no point in trying to use a feature that doesn't exist, even
though GCC is being nice by simply issuing a warning instead of an
error.
See:
http://developer.apple.com/releasenotes/DeveloperTools/RN-dyld/
index.html
In the section "Known Issues" it clearly states "No rpath support".
Cheers,
--
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 186 bytes --]
^ permalink raw reply
* Re: git: avoiding merges, rebasing
From: Bruno Haible @ 2007-10-03 23:01 UTC (permalink / raw)
To: Benoit SIGOURE; +Cc: bug-gnulib, git
In-Reply-To: <6C9F1445-8826-4E6F-A10C-290A57A4C826@lrde.epita.fr>
Benoit SIGOURE wrote:
> >>> Is there some shorthand for this process, such as a "git-recover"
> >>> command?
> >>
> >> you can run git fsck and look for
> >> a dangling commit (which can be inspected with git show <sha1>) where
> >> you are most likely to find your stash (unless you run git-gc or this
> >> sort of thing).
> >
> > Cool! This information would be worth mentioning in the git-stash
> > manual page or in an FAQ. It can be a real life-saver.
>
> True. Propose a patch :-)
Here's a proposed patch.
--- git-1.5.3.4/Documentation/git-stash.txt.bak 2007-10-03 21:44:53.000000000 +0200
+++ git-1.5.3.4/Documentation/git-stash.txt 2007-10-04 01:00:48.000000000 +0200
@@ -154,6 +154,28 @@
... continue hacking ...
----------------------------------------------------------------
+Recovering a lost stash::
+
+`git stash clear` clears all stashes without asking for confirmation.
+If you did not intend this, you can recover the lost changes by running
+`git fsck`, looking for dangling commits, and using `git show <commit>`.
++
+----------------------------------------------------------------
+... hack hack hack ...
+$ git stash
+... hack no coffee hack ...
+$ git stash clear
+... panic panic ...
+$ git fsck
+dangling commit f62c4fa05422fd4fb8610bdb02a7160121657893
+dangling commit 773024d2ffc33ac80baddcf2b673535b627af0da
+dangling commit 9e509390a2b26b562f3b008eacc65b6765d48339
+$ git show 773024d2ffc33ac80baddcf2b673535b627af0da
+$ git diff 773024d2ffc33ac80baddcf2b673535b627af0da > recovered.diff
+$ patch -p0 -R < recovered.diff
+... continue hacking ...
+----------------------------------------------------------------
+
SEE ALSO
--------
gitlink:git-checkout[1],
^ permalink raw reply
* Re: [PATCH] Add a simple option parser.
From: Pierre Habouzit @ 2007-10-03 23:11 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git, gitster
In-Reply-To: <1191447902-27326-1-git-send-email-krh@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1311 bytes --]
On Wed, Oct 03, 2007 at 09:45:01PM +0000, Kristian Høgsberg wrote:
> The option parser takes argc, argv, an array of struct option
> and a usage string. Each of the struct option elements in the array
> describes a valid option, its type and a pointer to the location where the
> value is written. The entry point is parse_options(), which scans through
> the given argv, and matches each option there against the list of valid
> options. During the scan, argv is rewritten to only contain the
> non-option command line arguments and the number of these is returned.
if we are going in that direction (and I believe it's a good one), we
should be sure that the model fits with other commands as well. And as I
said on IRC, I believe the most "horrible" (as in complex) option parser
in git is the one from git-grep.
A migration of git-grep on that API should be tried first. If this
works well enough, I believe that the rest of the git commands will be
migrated easily enough. (with maybe small addition to parse-option.[hc]
but the hardcore things should have been met with git-grep already I
think).
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] Be nice with compilers that do not support runtime paths at all.
From: Junio C Hamano @ 2007-10-03 23:18 UTC (permalink / raw)
To: Benoit Sigoure; +Cc: git
In-Reply-To: <1191450052-23619-1-git-send-email-tsuna@lrde.epita.fr>
Benoit Sigoure <tsuna@lrde.epita.fr> writes:
> diff --git a/Makefile b/Makefile
> index a1fe443..7c6c453 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -100,6 +100,9 @@ all::
> # that tells runtime paths to dynamic libraries;
> # "-Wl,-rpath=/path/lib" is used instead.
> #
> +# Define NO_RPATH if your dynamic loader doesn't support runtime paths at
> +# all.
> +#
> # Define USE_NSEC below if you want git to care about sub-second file mtimes
> # and ctimes. Note that you need recent glibc (at least 2.2.4) for this, and
> # it will BREAK YOUR LOCAL DIFFS! show-diff and anything using it will likely
Thanks for this part;
> @@ -507,6 +510,7 @@ ifeq ($(uname_S),Darwin)
> BASIC_LDFLAGS += -L/opt/local/lib
> endif
> endif
> + NO_RPATH = YesPlease
> endif
I'll let Darwin users to fight the defaults for this part out.
> @@ -521,7 +525,10 @@ ifndef NO_CURL
> ifdef CURLDIR
> # Try "-Wl,-rpath=$(CURLDIR)/$(lib)" in such a case.
> BASIC_CFLAGS += -I$(CURLDIR)/include
> - CURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$(lib) -lcurl
> + CURL_LIBCURL = -L$(CURLDIR)/$(lib) -lcurl
> +ifndef NO_RPATH
> + CURL_LIBCURL += $(CC_LD_DYNPATH)$(CURLDIR)/$(lib)
> +endif
> else
> CURL_LIBCURL = -lcurl
> endif
> @@ -539,7 +546,10 @@ endif
>
> ifdef ZLIB_PATH
> BASIC_CFLAGS += -I$(ZLIB_PATH)/include
> - EXTLIBS += -L$(ZLIB_PATH)/$(lib) $(CC_LD_DYNPATH)$(ZLIB_PATH)/$(lib)
> + EXTLIBS += -L$(ZLIB_PATH)/$(lib)
> +ifndef NO_RPATH
> + EXTLIBS += $(CC_LD_DYNPATH)$(ZLIB_PATH)/$(lib)
> +endif
> endif
> EXTLIBS += -lz
>
While these parts are ugly but correct, I think...
> @@ -547,7 +557,10 @@ ifndef NO_OPENSSL
> OPENSSL_LIBSSL = -lssl
> ifdef OPENSSLDIR
> BASIC_CFLAGS += -I$(OPENSSLDIR)/include
> - OPENSSL_LINK = -L$(OPENSSLDIR)/$(lib) $(CC_LD_DYNPATH)$(OPENSSLDIR)/$(lib)
> + OPENSSL_LINK = -L$(OPENSSLDIR)/$(lib)
> +ifndef NO_RPATH
> + OPENSSL_LINK = $(CC_LD_DYNPATH)$(OPENSSLDIR)/$(lib)
> +endif
> else
> OPENSSL_LINK =
> endif
this and the ICONV one are missing s/=/+=/.
If we do not care about supporting too old GNU make, we can do
this by first adding this near the top:
ifndef NO_RPATH
LINKER_PATH = -L$(1) $(CC_LD_DYNPATH)$(1)
else
LINKER_PATH = -L$(1)
endif
and then doing something like:
CURL_LIBCURL = $(call LINKER_PATH,$(CURLDIR)/$(lib))
OPENSSL_LINK = $(call LINKER_PATH,$(OPENSSLDIR)/$(lib))
to make it easier to read and less error prone.
^ permalink raw reply
* [PATCH] git-init: don't base core.filemode on the ability to chmod.
From: Martin Waitz @ 2007-10-03 23:19 UTC (permalink / raw)
To: git
In-Reply-To: <470388DC.4040504@viscovery.net>
At least on Linux the vfat file system honors chmod calls but does not
store them permanently (as there is no on-disk format for it).
So the filemode test which tries to chmod a file thinks that the file system
does support file modes which will result in problems later after the
file system got remounted.
Now we check both that new files are created without the executable bit
and that we can actually modify it with chmod.
Signed-off-by: Martin Waitz <tali@admingilde.org>
--- 8< ---
builtin-init-db.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
On Wed, Oct 03, 2007 at 02:19:40PM +0200, Johannes Sixt wrote:
> On Windows, we don't get an executable bit at all. Better use both
> heuristics, i.e. set core.filemode false if either one diagnoses an
> unreliable x-bit.
this should work better for Windows.
Previously I sent it only to Johannes and forgot to Cc the list.
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 763fa55..1d92916 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -247,7 +247,10 @@ static int create_default_files(const char *git_dir, const char *template_path)
filemode = TEST_FILEMODE;
if (TEST_FILEMODE && !lstat(path, &st1)) {
struct stat st2;
- filemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&
+ /* test that new files are not created with X bit */
+ filemode = !(st1.st_mode & S_IXUSR);
+ /* test that we can modify the X bit */
+ filemode &= (!chmod(path, st1.st_mode ^ S_IXUSR) &&
!lstat(path, &st2) &&
st1.st_mode != st2.st_mode);
}
--
1.5.3.3.8.g367dc7
--
Martin Waitz
^ permalink raw reply related
* Re: [PATCH] Don't checkout the full tree if avoidable
From: Eric Wong @ 2007-10-03 23:26 UTC (permalink / raw)
To: Steven Walter; +Cc: Junio C Hamano, git
In-Reply-To: <20071001131227.GA24494@dervierte>
Steven Walter <stevenrwalter@gmail.com> wrote:
> On Mon, Oct 01, 2007 at 04:08:55AM -0700, Eric Wong wrote:
> > Steven Walter wrote:
> > > One criticism of the patch: the trees_match function probably needs to
> > > be re-written. My SVN::Perl-foo is weak.
> >
> > Yep :)
> >
> > Steven:
> >
> > How does the following work for you? Which version of SVN do you have,
> > by the way? I just found a bug with the way SVN::Client::diff() is
> > exported for SVN 1.1.4, hence the SVN::Pool->new_default_sub usage.
>
> swalter@sentra:~% svn --version
> svn, version 1.3.2 (r19776)
>
> This version works great; seems to have exactly the same behavior as my
> patch. Verified that it still falls back to the do_update code when
> trees_match fails.
Thanks Steven.
Junio: can you please apply my version of Steven's patch? Thanks.
--
Eric Wong
^ permalink raw reply
* stgit: editing description of patch
From: Jon Smirl @ 2007-10-03 23:26 UTC (permalink / raw)
To: Git Mailing List
What is the right procedure for editing the various attributes of a
stgit patch? patch name, description, etc.... I just emailed a series
to myself and the titles/comments are all messed up.
On my box all of the patches have names -- stg series shows them. But
when I emailed them half of the patch didn't have the right subjects.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [PATCH] git-init: don't base core.filemode on the ability to chmod.
From: Johannes Schindelin @ 2007-10-03 23:54 UTC (permalink / raw)
To: Martin Waitz; +Cc: git
In-Reply-To: <20071003231941.GA20800@admingilde.org>
Hi,
On Thu, 4 Oct 2007, Martin Waitz wrote:
> - filemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&
> + /* test that new files are not created with X bit */
> + filemode = !(st1.st_mode & S_IXUSR);
> + /* test that we can modify the X bit */
> + filemode &= (!chmod(path, st1.st_mode ^ S_IXUSR) &&
Should that not be &&=?
Ciao,
Dscho
^ permalink raw reply
* [PATCH] builtin-apply: fix conversion error in strbuf series
From: Junio C Hamano @ 2007-10-04 0:53 UTC (permalink / raw)
To: git
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-apply.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 047a60d..05c6bc3 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2651,7 +2651,7 @@ static int apply_patch(int fd, const char *filename, int inaccurate_eof)
patch = xcalloc(1, sizeof(*patch));
patch->inaccurate_eof = inaccurate_eof;
- nr = parse_chunk(buf.buf + offset, buf.len, patch);
+ nr = parse_chunk(buf.buf + offset, buf.len - offset, patch);
if (nr < 0)
break;
if (apply_in_reverse)
--
1.5.3.4.1150.gbb28
^ permalink raw reply related
* Re: [PATCH] Be nice with compilers that do not support runtime paths at all.
From: Brian Gernhardt @ 2007-10-04 1:08 UTC (permalink / raw)
To: Steven Grimm; +Cc: Benoit Sigoure, git
In-Reply-To: <47041C7A.9090003@midwinter.com>
On Oct 3, 2007, at 6:49 PM, Steven Grimm wrote:
> Benoit Sigoure wrote:
>> On Darwin for instance, there is no -R or -Wl,-rpath thing to
>> fiddle with,
>> it's simply not supported by the dynamic loader. This patch
>> introduces a
>> NO_RPATH define which is enabled by default for Darwin.
>>
>
> I compile git on a MacBook Pro (OS X 10.4, gcc 4.0.1 build 5367
> from the normal Xcode install that comes on the OS install DVD) on
> a regular basis. The makefile works fine for me. I suspect there's
> something else going on here.
The rpath code is only used if you define one of the following options:
CURLDIR
ZLIB_PATH
OPENSSLDIR
ICONVDIR
The default Darwin options don't define any of these, it just relies
on finding those libraries in the library path (including /sw or /opt/
local if you have them installed).
~~ Brian G.
^ permalink raw reply
* Re: [PATCH] Be nice with compilers that do not support runtime paths at all.
From: Brian Gernhardt @ 2007-10-04 1:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Benoit Sigoure, git
In-Reply-To: <7vejgbdbyn.fsf@gitster.siamese.dyndns.org>
On Oct 3, 2007, at 7:18 PM, Junio C Hamano wrote:
> Benoit Sigoure <tsuna@lrde.epita.fr> writes:
>
>> @@ -507,6 +510,7 @@ ifeq ($(uname_S),Darwin)
>> BASIC_LDFLAGS += -L/opt/local/lib
>> endif
>> endif
>> + NO_RPATH = YesPlease
>> endif
>
> I'll let Darwin users to fight the defaults for this part out.
It makes sense, since Apple's gcc/ld/dyld doesn't use rpath.
~~ Brian
^ permalink raw reply
* Re: git-cvsserver test failures (still)
From: Frank Lichtenheld @ 2007-10-04 1:19 UTC (permalink / raw)
To: Brian Gernhardt; +Cc: Git Mailing List
In-Reply-To: <A78B5F62-4FCC-4650-9B0D-0F64FEDB8423@silverinsanity.com>
Hi.
Some investigations on this:
On Wed, Oct 03, 2007 at 03:50:18PM -0400, Brian Gernhardt wrote:
> This appears to be from git-cvsserver.perl:148-9:
>
> req_Root('root', $line) # reuse Root
> or die "E Invalid root $line \n";
>
> This fails the test suite because die() exits with code 255 (checked
> with "perl -e 'die'; echo $?"), which is outside what
> test_expect_failure accepts (see t/test-lib.sh:179).
>
> My questions become:
> 1) Why hasn't this hit anyone else?
die doesn't always quit with 255:
"exits with the current value of $! (errno). If $! is 0, exits with the
value of "($? >> 8)" (backtick ‘command‘ status). If "($? >> 8)" is 0,
exits with 255"
On my system $! is 9 for these cases and so it exits with exit code 9.
> 2) Is this where these tests are supposed to fail?
yes, IIRC
> 3) If it is, should the code be using print and exit 1 instead of die?
I think that would be the best solution, yes.
> 4) If not, should the test be altered to end with "|| false" or
> similar so the test passes?
Gruesse,
--
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/
^ permalink raw reply
* [PATCH] Use exit 1 instead of die when req_Root fails.
From: Brian Gernhardt @ 2007-10-04 1:43 UTC (permalink / raw)
To: git
In-Reply-To: <20071004011954.GK18024@planck.djpig.de>
This was causing test failures because die was exiting 255.
---
This finally takes care of my test failures.
git-cvsserver.perl | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 13dbd27..0d55fec 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -145,8 +145,10 @@ if ($state->{method} eq 'pserver') {
}
my $request = $1;
$line = <STDIN>; chomp $line;
- req_Root('root', $line) # reuse Root
- or die "E Invalid root $line \n";
+ unless (req_Root('root', $line)) { # reuse Root
+ print "E Invalid root $line \n";
+ exit 1;
+ }
$line = <STDIN>; chomp $line;
unless ($line eq 'anonymous') {
print "E Only anonymous user allowed via pserver\n";
--
1.5.3.4.203.gcc61a
^ permalink raw reply related
* Re: stgit: editing description of patch
From: Jon Smirl @ 2007-10-04 2:14 UTC (permalink / raw)
To: Git Mailing List
In-Reply-To: <9e4733910710031626kff59666y77ba9001c0fef907@mail.gmail.com>
On 10/3/07, Jon Smirl <jonsmirl@gmail.com> wrote:
> What is the right procedure for editing the various attributes of a
> stgit patch? patch name, description, etc.... I just emailed a series
> to myself and the titles/comments are all messed up.
Editing these is done with the stg refresh command.
>
> On my box all of the patches have names -- stg series shows them. But
> when I emailed them half of the patch didn't have the right subjects.
this is controled in the template files
>
> --
> Jon Smirl
> jonsmirl@gmail.com
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: WIP: asciidoc replacement
From: Sam Vilain @ 2007-10-04 4:13 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0710030506360.28395@racer.site>
Johannes,
Given other people have answered some points, I'll answer the rest.
Johannes Schindelin wrote:
>>> -- snip --
>>> #!/usr/bin/perl
>> Add -w for warnings, also use strict;
>
> <dumb>What does "use strict;" imply?</dumb>
Three things;
1. variables must be declared with 'my', 'our' or 'use var' before they
are used, to catch typos
2. when subroutine calls are found, they are checked to exist otherwise
they throw a compile-time error
3. force all dereferences to follow real references and not allow symbol
table access (don't worry about that ;-))
>>> sub handle_text {
>> this function acts on globals; make them explicit arguments to the
>> function.
>
> Actually, it resets the global $par. Should I rather make it a class?
Well, just channelling Dijkstra really. Functions should take all their
input as formal arguments rather than globals.
ie
sub handle_text {
my $par = shift;
}
If it really should be a global, it is perhaps best declared up front
with "our" or "use vars". "use strict" will force you to do one of these.
>> also consider making this a "tabular ternary" with the actions in
>> separate functions.
>>
>> ie
>>
>> $result = ( $par =~ /^\. /s ? $conv->do_enum($par) :
>> $par =~ /^\[verse\]/ ? $conv->do_verse($par) :
>> ... )
>
> I do not like that way... is it Perl standard to code like that?
It's in Perl Best Practices, but these are always suggestions and not
hard and fast rules. It just means that you have a big table of regex
-> function that you can quickly check rather than looking at a lot of
spaced out 'elsif's
>>> s/gitlink:([^\[ ]*)\[(\d+)\]/sprintf "%s",
>>> $conv->get_link($1, $2)/ge;
>>> # handle link:
>>> s/link:([^\[ ]*)\[(.+)\]/sprintf "%s",
>>> $conv->get_link($1, $2, 'external')/ge;
>> These REs suffer from LTS (Leaning Toothpick Syndrome). Consider using
>> s{foo}{bar} and adding the 'x' modifier to space out groups.
>
> I guess you mean the forward slash. Alas, that's what I'm used to, and
> I'd rather not change it unless forced to... lest I stop understanding my
> own code!
>
> (Besides, I did not find _any_ example showing why "x" should be useful.)
Before:
s/link:([^\[ ]*)\[(.+)\]/sprintf "%s",
$conv->get_link($1, $2, 'external')/ge;
After:
s{ link: ([^\[\040]*) \[(.+)\] }
{ sprintf "%s", $conv->get_link($1, $2, 'external') }gex;
>>> if ($self->{preamble_shown} == undef) {
>>> print '.\" disable hyphenation' . "\n"
>>> . '.nh' . "\n"
>>> . '.\" disable justification (adjust text to left'
>>> . ' margin only)' . "\n"
>>> . '.ad l' . "\n";
>> Using commas rather than "." will safe you a concat when printing to
>> filehandles, but that's a very small nit to pick :)
>
> Does that also work with older perl? IIRC there was some strange problem
> with my perl when lots of code in git.git was changed to using commata.
That should go back all the way to perl 4, if not earlier. If you're
assigning to a scalar, then you need to use concat. But very minor.
>> Hmm, that regex would not match for <<foo > bar>>, if you care you'd
>> need to write something like <<((?:[^>]+|>[^>])*)>>
>
> I'd rather leave it as is -- this script is not meant to grok all kind of
> sh*t. It is meant to make translating the docs as fast and uncumbersome
> as possible. Which will involve making the documentation more consistent
> (in and of itself something I rather like).
>
> So unless there comes a compelling reason, I'd rather leave it.
Sure, KISS.
> Another thing: if I want to add some documentation, what would be the
> common way to do it? =pod...=cut?
That's right. If you're an emacs user, in cperl-mode with abbrevs on
you type '=head1' and you'll get:
=head1 NAME
scriptname
=head1 SYNOPSIS
=head1 DESCRIPTION
=cut
See perlpod(3) for more!
Sam.
^ permalink raw reply
* stgit: lost all my patches again
From: Jon Smirl @ 2007-10-04 5:29 UTC (permalink / raw)
To: Git Mailing List
I believe this command did it:
stg refresh -p pcm030_bsp_powerpc -e
that patch whitespace errors in it
I edited the description and removed the last line
it was 10th patch down in the stack
for some reason the refresh from that command didn't close.
Then stg pushed all the patches back after the edit and they got
included into that patch.
stg should definitely have an assert that the previous patch is closed
before auto pushing.
There is a bug somewhere that caused that edit refresh not to get closed.
I am using stg version:
5ae6fcce77a29221e15f6a4e8348bd4276960ba1
It might also be good to make a log that lets me rollback commands.
All of the info is still in the system.
This refresh failed to close:
jonsmirl@terra:~/mpc5200b$ stg log pcm030_bsp_powerpc
95a8e03b [push ] Thu Oct 4 00:27:53 2007 -0400
6094a0a5 [refresh] Thu Oct 4 00:00:41 2007 -0400
0c4f5480 [push(f)] Wed Oct 3 22:10:38 2007 -0400
9a685ae9 [push(f)] Wed Oct 3 22:05:47 2007 -0400
This push ended up in the bsp patch:
jonsmirl@terra:~/mpc5200b$ stg log mpc52xx_restart
801962ed [push ] Thu Oct 4 00:33:42 2007 -0400
9e0c7417 [push ] Thu Oct 4 00:27:53 2007 -0400
37822491 [push ] Thu Oct 4 00:00:41 2007 -0400
9aff07ff [push(f)] Wed Oct 3 22:10:38 2007 -0400
c41c9cb3 [push(f)] Wed Oct 3 22:05:47 2007 -0400
and so one for nine more patches.
The ten messed up patches still have their descriptions, the are just
missing the changes.
---------------------------------
After an export stg puts the patches into patches-m25
Looking back in my command logs I had done:
stg export
cd patches-m25
grep for some things
stg refresh -p pcm030_bsp_powerpc -e
I did the 'stg refresh' from a directory that was not being tracked by git.
It is in the .gitignore list. This appears to be the root of the problem.
--
Jon Smirl
jonsmirl@gmail.com
^ 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