* 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
* 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] 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
* [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] 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
* 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: 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: 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: [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
* [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: 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
* 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: [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 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] 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
* [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] 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] Port builtin-add.c to use the new option parser.
From: Kristian Høgsberg @ 2007-10-03 21:45 UTC (permalink / raw)
To: git; +Cc: gitster, Kristian Høgsberg
In-Reply-To: <1191447902-27326-1-git-send-email-krh@redhat.com>
Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
builtin-add.c | 64 ++++++++++++++++++--------------------------------------
1 files changed, 21 insertions(+), 43 deletions(-)
diff --git a/builtin-add.c b/builtin-add.c
index 966e145..66fd99d 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -13,6 +13,7 @@
#include "commit.h"
#include "revision.h"
#include "run-command.h"
+#include "parse-options.h"
static const char builtin_add_usage[] =
"git-add [-n] [-v] [-f] [--interactive | -i] [-u] [--refresh] [--] <filepattern>...";
@@ -160,21 +161,30 @@ static struct lock_file lock_file;
static const char ignore_error[] =
"The following paths are ignored by one of your .gitignore files:\n";
+static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;
+static int add_interactive = 0;
+
+static struct option builtin_add_options[] = {
+ { OPTION_BOOLEAN, "interactive", 'i', &add_interactive },
+ { OPTION_BOOLEAN, NULL, 'n', &show_only },
+ { OPTION_BOOLEAN, NULL, 'f', &ignored_too },
+ { OPTION_BOOLEAN, NULL, 'v',&verbose },
+ { OPTION_BOOLEAN, NULL, 'u',&take_worktree_changes },
+ { OPTION_BOOLEAN, "refresh", 0, &refresh_only }
+};
+
int cmd_add(int argc, const char **argv, const char *prefix)
{
int i, newfd;
- int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;
const char **pathspec;
struct dir_struct dir;
- int add_interactive = 0;
- for (i = 1; i < argc; i++) {
- if (!strcmp("--interactive", argv[i]) ||
- !strcmp("-i", argv[i]))
- add_interactive++;
- }
+ i = parse_options(argc, argv, builtin_add_options,
+ ARRAY_SIZE(builtin_add_options),
+ builtin_add_usage);
+
if (add_interactive) {
- if (argc != 2)
+ if (i > 0)
die("add --interactive does not take any parameters");
exit(interactive_add());
}
@@ -183,51 +193,19 @@ int cmd_add(int argc, const char **argv, const char *prefix)
newfd = hold_locked_index(&lock_file, 1);
- for (i = 1; i < argc; i++) {
- const char *arg = argv[i];
-
- if (arg[0] != '-')
- break;
- if (!strcmp(arg, "--")) {
- i++;
- break;
- }
- if (!strcmp(arg, "-n")) {
- show_only = 1;
- continue;
- }
- if (!strcmp(arg, "-f")) {
- ignored_too = 1;
- continue;
- }
- if (!strcmp(arg, "-v")) {
- verbose = 1;
- continue;
- }
- if (!strcmp(arg, "-u")) {
- take_worktree_changes = 1;
- continue;
- }
- if (!strcmp(arg, "--refresh")) {
- refresh_only = 1;
- continue;
- }
- usage(builtin_add_usage);
- }
-
if (take_worktree_changes) {
if (read_cache() < 0)
die("index file corrupt");
- add_files_to_cache(verbose, prefix, argv + i);
+ add_files_to_cache(verbose, prefix, argv);
goto finish;
}
- if (argc <= i) {
+ if (i == 0) {
fprintf(stderr, "Nothing specified, nothing added.\n");
fprintf(stderr, "Maybe you wanted to say 'git add .'?\n");
return 0;
}
- pathspec = get_pathspec(prefix, argv + i);
+ pathspec = get_pathspec(prefix, argv);
if (refresh_only) {
refresh(verbose, pathspec);
--
1.5.2.5
^ permalink raw reply related
* Re: Linking with -R (rpath) not supported on Darwin
From: Junio C Hamano @ 2007-10-03 21:41 UTC (permalink / raw)
To: Benoit SIGOURE; +Cc: git list
In-Reply-To: <4D954ADB-E66E-43CA-87EE-7522FFA87370@lrde.epita.fr>
Benoit SIGOURE <tsuna@lrde.epita.fr> writes:
> 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.
As I do not have an access to a Darwin box (nor anybody sent me
a free Mac yet), I do not have any interest in fixing it myself
nor more importantly any means to verify the result. That makes
it _your_ build process that should be more careful ;-).
You know where -R is coming from and can find out what options
_your_ platform wants, so why not send in a patch _before_
complaining?
^ permalink raw reply
* Re: [PATCH] Add test case for ls-files --with-head
From: Johannes Schindelin @ 2007-10-03 21:39 UTC (permalink / raw)
To: Jeff King
Cc: Carl Worth, Johannes Sixt, Junio C Hamano, Keith Packard,
Git Mailing List
In-Reply-To: <20071003202157.GA28043@coredump.intra.peff.net>
Hi,
On Wed, 3 Oct 2007, Jeff King wrote:
> > for i in {1,2,3,4,5}{0,1,2,3,4,5,6,7,8,9}
> > do
> > ...
> > done
>
> $ dash
> $ 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}.
Ciao,
Dscho
^ permalink raw reply
* Re: making "git stash" safer to use
From: Junio C Hamano @ 2007-10-03 21:36 UTC (permalink / raw)
To: Bruno Haible; +Cc: git, Benoit SIGOURE, Eric Blake
In-Reply-To: <200710032331.41385.bruno@clisp.org>
Bruno Haible <bruno@clisp.org> writes:
>> While we're at it, I wish 'git stash clear' would take an optional
>> argument that says which stash(es) to clear, rather than blindly clearing
>> the entire stash.
>
> It would help if git would store which of the stashes were applied since
> they were created and which were not. A stash that was not yet applied must
> be considered "precious", whereas a stash that was applied is redundant,
> right?
Wrong. I would say all stash entries are precious unless you
explicitly say "I'm done with it". The problem is that we do
not have a way to say it explicitly.
^ permalink raw reply
* Re: size_t vs "unsigned long"
From: Florian Weimer @ 2007-10-03 21:36 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Junio C Hamano, git
In-Reply-To: <20071003204801.GC28188@artemis.corp>
* Pierre Habouzit:
> Well, afaict, on every linux archs I know of, unsigned longs and
> size_t are the same.
IIRC, 64-bit Windows uses 64-bit points (duh) and hence a 64-bit
size_t, but still has got 32-bit longs. Documentation is a bit sparse
on this matter (because you are supposed to use LONG, DWORD and
friends anyway).
^ permalink raw reply
* Re: size_t vs "unsigned long"
From: Pierre Habouzit @ 2007-10-03 21:36 UTC (permalink / raw)
To: Jan Wielemaker; +Cc: Junio C Hamano, git
In-Reply-To: <200710032320.00263.wielemak@science.uva.nl>
[-- Attachment #1: Type: text/plain, Size: 1834 bytes --]
On Wed, Oct 03, 2007 at 09:19:59PM +0000, Jan Wielemaker wrote:
> On Wednesday 03 October 2007 22:48, Pierre Habouzit wrote:
> > On Wed, Oct 03, 2007 at 08:30:04PM +0000, Junio C Hamano wrote:
> > > Traditionally, inside git, we have used the length of things
> > > with "unsigned long" for pretty much anything, except where we
> > > wanted the length exactly sized we used int32_t, uint64_t and
> > > friends.
> > >
> > > A few places pass pointer to unsigned long as the second
> > > parameter to strbuf_detach(), triggering type mismatch warnings.
> > > An easy way out is to change strbuf_detach() to take a pointer
> > > to ulong but I think it is going backwards. Most places that
> > > use "unsigned long" can safely be converted (and made more
> > > correct) to use size_t.
> >
> > Well, afaict, on every linux archs I know of, unsigned longs and
> > size_t are the same. Though, I don't know if that holds for the msys
> > port, and if that does not holds, then a s/unsigned long/size_t/ would
> > help them. Else, for consistency sake, I believe the change is a good
> > one.
>
> Surely on the Microsoft 64-bit compilers size_t is 64-bits and long is
> 32-bits. Don't blame me, I'm just the messenger that learned the hard
> way ...
Yeah, I've been wondering, and it's the information I had. well, the
information I had is that sizeof(size_t) is 4 on win32, and 8 on win64,
OTOH (and this one I'm sure), on windows, longs are 32bits on both (32
and 64 bits ABIs).
So replacing unsigned long with size_t's will help the msys port,
hence I had some insight that this could prove useful, now I'm sure :)
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Linking with -R (rpath) not supported on Darwin
From: Benoit SIGOURE @ 2007-10-03 21:34 UTC (permalink / raw)
To: git list
[-- Attachment #1: Type: text/plain, Size: 416 bytes --]
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.
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
* making "git stash" safer to use
From: Bruno Haible @ 2007-10-03 21:31 UTC (permalink / raw)
To: git; +Cc: Benoit SIGOURE, Eric Blake
In-Reply-To: <47023699.3080606@byu.net>
Hi,
Through a simple typo I lost modifications to 20 files:
> >>> $ git stash
> >>> $ git pull
> >>> $ git stash apply
> >>> $ git stash clean # typo!
> >>> $ git stash clear # fatal correction to typo!
It is just too easy to lose your modifications by using "git stash".
Eric Blake further says:
> While we're at it, I wish 'git stash clear' would take an optional
> argument that says which stash(es) to clear, rather than blindly clearing
> the entire stash.
It would help if git would store which of the stashes were applied since
they were created and which were not. A stash that was not yet applied must
be considered "precious", whereas a stash that was applied is redundant,
right?
According these lines, how about
1) changing "git stash clear" to remove only the redundant stashes,
(or alternatively: let it fail if there is at least one precious stash),
2) adding an option -f, so that "git stash -f clear" clears all stashes,
including the precious ones.
The rationale is that humans are bad at remembering the state of something.
Therefore instead of having a command that is commonly used in one state
and dangerous in the other state, better have two different commands - one
for the common case, and one for the dangerous one. Like "rm" and "rm -f".
Bruno
^ 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