* Re: [PATCH 1/2] Have a filter_start/filter_end API.
From: Pierre Habouzit @ 2007-10-07 14:53 UTC (permalink / raw)
To: Alex Riesen; +Cc: Linus Torvalds, Junio C Hamano, git
In-Reply-To: <20071006090621.GB2711@steel.home>
[-- Attachment #1: Type: text/plain, Size: 5549 bytes --]
On Sat, Oct 06, 2007 at 09:06:21AM +0000, Alex Riesen wrote:
> Pierre Habouzit, Fri, Oct 05, 2007 22:19:30 +0200:
> > Those are helpers to build functions that transform a buffer into a
> > strbuf, allowing the "buffer" to be taken from the destination buffer.
>
> They are horrible. And very specialized for these "filter" routines.
> To the point where I would move them into the file where they are used
> (convert.c only, isn't it?)
For now they are, but moving them into convert.c is at the very least
awkward.
> If you continue to insist the code is generic enough to justify its
> residence in strbuf.c, continue reading.
>
> First off, what was wrong with dumb
>
> void strbuf_make_room(struct strbuf *, size_t newsize);
>
> again?
If newsize is >= sb->alloc then the area is reallocated, the pointer
may move, and the "src" pointer would then be invalidated.
> > +void *strbuf_start_filter(struct strbuf *sb, const char *src, ssize_t hint)
> > +{
> > + if (src < sb->buf || src >= sb->buf + sb->alloc) {
> > + if (hint > 0 && (size_t)hint >= sb->alloc)
> > + strbuf_grow(sb, hint - sb->len);
> > + return NULL;
> > + }
> > +
> > + if (hint < 0)
> > + return strbuf_detach(sb, NULL);
> what is that for? Why can't the caller just use strbuf_detach? (He
> already has to pass negative hint somehow, which should be a concious
> action).
The idea is to have a unified API to deal with both the cases where
the filtering is known not to work in place by the caller, or for the
cases where it could if enough space is allocated but that a realloc is
needed.
> > + if ((size_t)hint >= sb->alloc) {
> > + void *tmp = strbuf_detach(sb, NULL);
> > + strbuf_grow(sb, hint);
> > + return tmp;
> > + }
> > +
> > + return NULL;
> > +}
>
> How can one know when it sb is safe to use after strbuf_end_filter?
We could document it, that's not an issue.
> It is for the first "if", for example. free() wont free the buf in sb.
> Oh, right, one can check if returned pointer !NULL. Which just adds
> more code to handle your API.
I don't get that part. free(NULL) is totally ok.
> What actually happens to sb? Is it detached? Is it reallocated?
> When it is detached and when it is reallocated?
It is detached if the filter does not works in place (caller says that
with '-1' as a hint) or if it works in place but needs a buffer
reallocation.
> Why is the returned pointer useful only for giving it to
> strbuf_end_filter?
Because the filter works on a {src, len} buffer, that points into the
buffer that we must free later in strbuf_end_filter.
> Take for example your change in crlf_to_git:
> @@ -85,6 +85,7 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
> {
> struct text_stat stats;
> char *dst;
> + void *tmp;
>
> if ((action == CRLF_BINARY) || !auto_crlf || !len)
> return 0;
> @@ -110,9 +111,7 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
> return 0;
> }
>
> - /* only grow if not in place */
> - if (strbuf_avail(buf) + buf->len < len)
> - strbuf_grow(buf, len - buf->len);
> + tmp = strbuf_start_filter(buf, src, len);
> dst = buf->buf;
> if (action == CRLF_GUESS) {
> /*
> @@ -133,13 +132,14 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
> } while (--len);
> }
> strbuf_setlen(buf, dst - buf->buf);
> + strbuf_end_filter(tmp);
> return 1;
> }
>
> And try to rewrite it with the strbuf_make_room:
>
> @@ -110,9 +111,7 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
> return 0;
> }
>
> - /* only grow if not in place */
> - if (strbuf_avail(buf) + buf->len < len)
> - strbuf_grow(buf, len - buf->len);
> + strbuf_make_room(buf, len);
> dst = buf->buf;
> if (action == CRLF_GUESS) {
> /*
>
> The change looks rather simple
but is wrong because you may just break "src". It's not the case here
because len always fits, but it's IMHO the kind of knowledge of the
internals of strbufs we should not rely on.
> > +/*----- filter API -----*/
> > +extern void *strbuf_start_filter(struct strbuf *, const char *, ssize_t);
> > +extern void strbuf_end_filter(void *p);
>
> I find the naming very confusing: what filtering takes place where?
> If strbuf_end_filter is just free, why is it needed at all? For the
> sake of wrapping free()?
Those are not filters, but help writing filters that would have this
prototype:
int some_filter(const char *src, size_t len, struct strbuf *dst);
Allowing [src..src+len[ to be a subpart of `dst'. The usual semantics
of such filters is that it returns 0 if it did nothing, 1 if it wrote in
dst.
This way, you can write filter_all being filter1, filter2, filter3
done one after the other this way:
int filter_all(const char *src, size_t len, struct strbuf *dst) {
int res = 0;
res |= filter1(src, len, dst);
if (res) {
src = dst->buf;
len = dst->len;
}
res |= filter2(src, len, dst);
if (res) {
src = dst->buf;
len = dst->len;
}
return res | filter3(src, len, dst);
}
Note that I did not created this semantics, it was how convert.c
worked already, in a even more convoluted way before.
--
·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: Question about "git commit -a"
From: Dmitry Potapov @ 2007-10-07 14:50 UTC (permalink / raw)
To: Marko Macek
Cc: Kristian Høgsberg, Andreas Ericsson, Paolo Ciarrocchi,
Johannes Schindelin, Nguyen Thai Ngoc Duy, Wincent Colaiuta,
Git Mailing List, andyparkins, torvalds
In-Reply-To: <470878CB.2010609@gmx.net>
On 10/7/07, Marko Macek <Marko.Macek@gmx.net> wrote:
> Dmitry Potapov wrote:
> > You don't. Even with 'commit -a' there is no guarantee that the
> > result will compile, because you can forget to add a new file.
>
> Actually, it would be a good idea for commit to report an error if there
> are any new files that have not been 'added' or 'ignored'
If it is good for you, you can add this check to your pre-commit hook.
However, I don't like your idea at all. Sometimes, you do want to have
some file that is not 'added' or 'ignored' as a reminder that you have
something else to do. IMHO, git acts in the most reasonable way in this
respect. When you say 'commit -a' and you have some files not added,
it will show you all untracked files in your editor when you type a
commit message. So, it is more difficult to forget to add a new file
with git than with many other version control systems.
> (or even
> if there are missing files that have not been 'deleted'.
Actually, 'commit -a' will automatically delete all missing files.
IMHO, it is the right thing to do.
Dmitry
^ permalink raw reply
* Re: [PATCH] Make strbuf_cmp inline, constify its arguments and optimize it a bit
From: Pierre Habouzit @ 2007-10-07 14:39 UTC (permalink / raw)
To: Timo Hirvonen; +Cc: Alex Riesen, git, Junio C Hamano
In-Reply-To: <20071007172425.bb691da9.tihirvon@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 933 bytes --]
On Sun, Oct 07, 2007 at 02:24:25PM +0000, Timo Hirvonen wrote:
> Alex Riesen <raa.lkml@gmail.com> wrote:
>
> > +static inline int strbuf_cmp(const struct strbuf *a, const struct strbuf *b)
> > +{
> > + int len = a->len < b->len ? a->len: b->len;
> > + int cmp = memcmp(a->buf, b->buf, len);
> > + if (cmp)
> > + return cmp;
> > + return a->len < b->len ? -1: a->len != b->len;
> > +}
>
> strbuf->buf is always non-NULL and NUL-terminated so you could just do
>
> static inline int strbuf_cmp(const struct strbuf *a, const struct strbuf *b)
> {
> int len = a->len < b->len ? a->len : b->len;
> return memcmp(a->buf, b->buf, len + 1);
> }
doesn't work, because a buffer can have (in some very specific cases)
an embeded NUL.
--
·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] Make strbuf_cmp inline, constify its arguments and optimize it a bit
From: David Kastrup @ 2007-10-07 14:24 UTC (permalink / raw)
To: git
In-Reply-To: <20071007140052.GA3260@steel.home>
Alex Riesen <raa.lkml@gmail.com> writes:
> It is definitely less code (also object code). It is not always
> measurably faster (but mostly is).
> -int strbuf_cmp(struct strbuf *a, struct strbuf *b)
> -{
> - int cmp;
> - if (a->len < b->len) {
> - cmp = memcmp(a->buf, b->buf, a->len);
> - return cmp ? cmp : -1;
> - } else {
> - cmp = memcmp(a->buf, b->buf, b->len);
> - return cmp ? cmp : a->len != b->len;
> - }
> -}
> -
> +static inline int strbuf_cmp(const struct strbuf *a, const struct strbuf *b)
> +{
> + int len = a->len < b->len ? a->len: b->len;
> + int cmp = memcmp(a->buf, b->buf, len);
> + if (cmp)
> + return cmp;
> + return a->len < b->len ? -1: a->len != b->len;
> +}
My guess is that you are conflating two issues about speed here: the
inlining will like speed the stuff up. But having to evaluate the
(a->len < b->len) comparison twice will likely slow it down.
So if you do any profiling, you should do it on both separate angles
of this patch.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: [PATCH] Make strbuf_cmp inline, constify its arguments and optimize it a bit
From: Timo Hirvonen @ 2007-10-07 14:24 UTC (permalink / raw)
To: Alex Riesen; +Cc: git, Junio C Hamano, Pierre Habouzit
In-Reply-To: <20071007140052.GA3260@steel.home>
Alex Riesen <raa.lkml@gmail.com> wrote:
> +static inline int strbuf_cmp(const struct strbuf *a, const struct strbuf *b)
> +{
> + int len = a->len < b->len ? a->len: b->len;
> + int cmp = memcmp(a->buf, b->buf, len);
> + if (cmp)
> + return cmp;
> + return a->len < b->len ? -1: a->len != b->len;
> +}
strbuf->buf is always non-NULL and NUL-terminated so you could just do
static inline int strbuf_cmp(const struct strbuf *a, const struct strbuf *b)
{
int len = a->len < b->len ? a->len : b->len;
return memcmp(a->buf, b->buf, len + 1);
}
^ permalink raw reply
* Re: [StGIT PATCH] Better diagnostic for wrong branch configuration.
From: Yann Dirson @ 2007-10-07 14:17 UTC (permalink / raw)
To: Karl Hasselström; +Cc: Catalin Marinas, git
In-Reply-To: <20071007131417.GA28492@diana.vm.bytemark.co.uk>
On Sun, Oct 07, 2007 at 03:14:17PM +0200, Karl Hasselström wrote:
> On 2007-10-05 22:44:52 +0200, Yann Dirson wrote:
>
> > If the branch.*.merge parameter does not name a valid remote head,
> > stgit would not rebase after a fetch, and would write instead
> > 'Rebasing to "None" ... done'.
> >
> > This patch makes this situation an error and tells the user what to
> > fix in his repo configuration.
>
> Good. Sign-off?
Oops, sorry - "Signed-off-by: Yann Dirson <ydirson@altern.org>"
> > - raise GitException, "StGit does not support multiple FETCH_HEAD"
> > + raise GitException, 'StGit does not support multiple FETCH_HEAD'
>
> Unrelated quote fixup. No big deal, though.
Right - did not seem to warrant a patch of its own :)
Best regards,
--
Yann
^ permalink raw reply
* [PATCH] Make strbuf_cmp inline, constify its arguments and optimize it a bit
From: Alex Riesen @ 2007-10-07 14:00 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Pierre Habouzit
In-Reply-To: <1190625904-22808-2-git-send-email-madcoder@debian.org>
It is definitely less code (also object code). It is not always
measurably faster (but mostly is).
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
strbuf.c | 12 ------------
strbuf.h | 9 ++++++++-
2 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index f4201e1..215837b 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -58,18 +58,6 @@ void strbuf_rtrim(struct strbuf *sb)
sb->buf[sb->len] = '\0';
}
-int strbuf_cmp(struct strbuf *a, struct strbuf *b)
-{
- int cmp;
- if (a->len < b->len) {
- cmp = memcmp(a->buf, b->buf, a->len);
- return cmp ? cmp : -1;
- } else {
- cmp = memcmp(a->buf, b->buf, b->len);
- return cmp ? cmp : a->len != b->len;
- }
-}
-
void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,
const void *data, size_t dlen)
{
diff --git a/strbuf.h b/strbuf.h
index 9b9e861..3116387 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -78,7 +78,14 @@ static inline void strbuf_setlen(struct strbuf *sb, size_t len) {
/*----- content related -----*/
extern void strbuf_rtrim(struct strbuf *);
-extern int strbuf_cmp(struct strbuf *, struct strbuf *);
+static inline int strbuf_cmp(const struct strbuf *a, const struct strbuf *b)
+{
+ int len = a->len < b->len ? a->len: b->len;
+ int cmp = memcmp(a->buf, b->buf, len);
+ if (cmp)
+ return cmp;
+ return a->len < b->len ? -1: a->len != b->len;
+}
/*----- add data in your buffer -----*/
static inline void strbuf_addch(struct strbuf *sb, int c) {
--
1.5.3.4.223.g78587
^ permalink raw reply related
* Re: [StGIT PATCH] Better diagnostic for wrong branch configuration.
From: Karl Hasselström @ 2007-10-07 13:14 UTC (permalink / raw)
To: Yann Dirson; +Cc: Catalin Marinas, git
In-Reply-To: <20071005204452.30902.60246.stgit@gandelf.nowhere.earth>
On 2007-10-05 22:44:52 +0200, Yann Dirson wrote:
> If the branch.*.merge parameter does not name a valid remote head,
> stgit would not rebase after a fetch, and would write instead
> 'Rebasing to "None" ... done'.
>
> This patch makes this situation an error and tells the user what to
> fix in his repo configuration.
Good. Sign-off?
> - raise GitException, "StGit does not support multiple FETCH_HEAD"
> + raise GitException, 'StGit does not support multiple FETCH_HEAD'
Unrelated quote fixup. No big deal, though.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: Question about "git commit -a"
From: Wincent Colaiuta @ 2007-10-07 12:26 UTC (permalink / raw)
To: Marko Macek
Cc: Kristian Høgsberg, Andreas Ericsson, Paolo Ciarrocchi,
Johannes Schindelin, Nguyen Thai Ngoc Duy, Git Mailing List
In-Reply-To: <47067F68.2080709@gmx.net>
El 5/10/2007, a las 20:16, Marko Macek escribió:
> In CVS and subversion (which has nicer working-copy command line
> interface IMHO),
> I simply make a copy of the working copy, revert the non-commitable
> parts, build,
> commit the minor changes, and then update the first copy. For
> larger projects,
> where this can be slow, I use diff/revert/patch.
This sounds painful compared to Dmitry's method (pasted below) if you
care about all published changes being buildable and passing all the
tests...
El 5/10/2007, a las 23:10, Dmitry Potapov escribió:
> IMHO, the best practice is to recompile everything step-wise in
> a clean directory before you are going to publish your changes.
> It can be done automatically by script, while you do something
> useful, like reading this mailing-list :)
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH] git-svn: respect Subversion's [auth] section configuration values
From: Eygene Ryabinkin @ 2007-10-07 10:14 UTC (permalink / raw)
To: Eric Wong, Sam Vilain; +Cc: git
In-Reply-To: <20071007032241.GG14972@hand.yhbt.net>
Eric, Sam, good day.
Sat, Oct 06, 2007 at 08:22:41PM -0700, Eric Wong wrote:
> Eygene Ryabinkin <rea-git@codelabs.ru> wrote:
> > Parameters 'store-passwords' and 'store-auth-creds' from Subversion's
> > configuration (~/.subversion/config) were not respected. This was
> > fixed: the default values for these parameters are set to 'yes' to
> > follow Subversion behaviour.
>
> Thanks.
You're welcome ;))
Sun, Oct 07, 2007 at 02:24:43PM +1300, Sam Vilain wrote:
> Eygene Ryabinkin wrote:
> > Parameters 'store-passwords' and 'store-auth-creds' from Subversion's
> > configuration (~/.subversion/config) were not respected. This was
> > fixed: the default values for these parameters are set to 'yes' to
> > follow Subversion behaviour.
> >
>
> I saw this in the svn api before. It really is a strange API, requiring
> the user to get things like this right.
Yes, the need to parse the configuration and set some flags is
rather strange. Looks like nobody cared to stuff the code like
I had added to the configuration file parsing routines.
> You can use no warnings 'once';
Great, thanks for the pointer! Eric, do you want me to produce
another patch or you'll correct mine?
--
Eygene
^ permalink raw reply
* Re: git-push [--all] and tags
From: martin f krafft @ 2007-10-07 9:36 UTC (permalink / raw)
To: git discussion list; +Cc: Linus Torvalds, Sam Vilain
In-Reply-To: <alpine.LFD.0.999.0710061814310.23684@woody.linux-foundation.org>
[-- Attachment #1: Type: text/plain, Size: 1097 bytes --]
also sprach Linus Torvalds <torvalds@linux-foundation.org> [2007.10.07.0226 +0100]:
> Well, I agrewe with Martin, and I think you missed his point.
(he did)
> Hmm? I do agree with "git push" as it now stands has a lot of warts,
> although I don't really agree with the people who want to change the
> default behavior.
So am I right if I say that all the logic should really be happening
in send-pack and that push is really just an interface when it comes
to selecting the refs to push, so it should basically feed through
the options and refs, meaning that send-pack should get --tags and
--shared as well?
Or should push enumerate all refs needed and pass them directly to
send-pack, effectively making send-pack's --all option obsolete?
--
martin; (greetings from the heart of the sun.)
\____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
"time flies like an arrow. fruit flies like a banana."
-- groucho marx
(panda eats shoots and leaves)
spamtraps: madduck.bogus@madduck.net
[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Repository specific config file
From: Pekka Riikonen @ 2007-10-07 8:37 UTC (permalink / raw)
To: git
Hello,
Has there been any discussion or considerations adding a repository
specific config file that would be delivered to all cloned repositories
automatically? This would allow the publisher of the repository to set up
some default settings to all developers cloning the repository. This
could be useful to set up things like default aliases to ease up the work,
to set that all remote branches are tracked, to list the configurations of
submodules (so that people don't have to run submodule init), etc.
User should still be able to add configurations that overlap with the
remote configuration and user should be able to ignore everything that
came from the parent. Using ~/.gitconfig and /etc/gitconfig doesn't
always work well when the developers don't have access to the remote
machine (other than to clone the repository). Maybe the simplest solution
would be to just add .gitconfig to the repository itself.
Pekka
________________________________________________________________________
Pekka Riikonen priikone at silcnet.org
Secure Internet Live Conferencing (SILC) http://silcnet.org/
^ permalink raw reply
* Re: Question about "git commit -a"
From: Marko Macek @ 2007-10-07 6:12 UTC (permalink / raw)
To: Dmitry Potapov
Cc: Kristian Høgsberg, Andreas Ericsson, Paolo Ciarrocchi,
Johannes Schindelin, Nguyen Thai Ngoc Duy, Wincent Colaiuta,
Git Mailing List, andyparkins, torvalds
In-Reply-To: <20071005211011.GB25125@potapov>
Dmitry Potapov wrote:
> You don't. Even with 'commit -a' there is no guarantee that the
> result will compile, because you can forget to add a new file.
Actually, it would be a good idea for commit to report an error if there
are any new files that have not been 'added' or 'ignored' (or even
if there are missing files that have not been 'deleted'.
Perhaps I'll add this to my git wrapper scripts.
Even for normal git-commit it might be nice to report an error if there are any
unstaged files unless -a or -i option is specified.
Mark
^ permalink raw reply
* Re: Many gits are offline this week
From: Steven Grimm @ 2007-10-07 4:59 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.64.0710050240460.4174@racer.site>
Johannes Schindelin wrote:
> While at it, we thought that we'd have a little git-together on Sunday, a
> few hours before we're (or at least I'm) sober again from Saturday night.
> So everybody who is in the area of San Jose: Sunday is Talk Like A Git
> Day.
>
Here are the details for anyone who can show up:
Time: 12 noon
Place: Gordon Biersch Brewery, 640 Emerson St., Palo Alto
Web:
http://www.gordonbiersch.com/restaurants/index.php?pg=location&sub=loc&location_id=15
Facebook event page: http://www.facebook.com/event.php?eid=5037248613
(obviously not required, but it might give other people some idea who to
expect)
Hope to see some folks there!
-Steve
^ permalink raw reply
* Re: [PATCH] git-svn: respect Subversion's [auth] section configuration values
From: Eric Wong @ 2007-10-07 3:24 UTC (permalink / raw)
To: Sam Vilain; +Cc: Eygene Ryabinkin, git
In-Reply-To: <4708355B.4090403@vilain.net>
Sam Vilain <sam@vilain.net> wrote:
> Eygene Ryabinkin wrote:
> > + # The usage of $SVN::_Core::SVN_CONFIG_* variables
> > + # produces warnings that variables are used only once.
> > + # I had not found the better way to shut them up, so
> > + # warnings are disabled in this block.
> > + no warnings;
> >
>
> You can use no warnings 'once';
Interesting, I should use that globally in git-svn and get rid of the
$kill_stupid_warnings variable :)
--
Eric Wong
^ permalink raw reply
* Re: [PATCH] git-svn: respect Subversion's [auth] section configuration values
From: Eric Wong @ 2007-10-07 3:22 UTC (permalink / raw)
To: Eygene Ryabinkin; +Cc: git
In-Reply-To: <20071006185719.GA3943@void.codelabs.ru>
Eygene Ryabinkin <rea-git@codelabs.ru> wrote:
> Parameters 'store-passwords' and 'store-auth-creds' from Subversion's
> configuration (~/.subversion/config) were not respected. This was
> fixed: the default values for these parameters are set to 'yes' to
> follow Subversion behaviour.
Thanks.
> Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>
Acked-by: Eric Wong <normalperson@yhbt.net>
> ---
> git-svn.perl | 23 +++++++++++++++++++++++
> 1 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index 484b057..f7ef421 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -3051,6 +3051,29 @@ sub new {
> ]);
> my $config = SVN::Core::config_get_config($config_dir);
> $RA = undef;
> + my $dont_store_passwords = 1;
> + my $conf_t = ${$config}{'config'};
> + {
> + # The usage of $SVN::_Core::SVN_CONFIG_* variables
> + # produces warnings that variables are used only once.
> + # I had not found the better way to shut them up, so
> + # warnings are disabled in this block.
> + no warnings;
> + if (SVN::_Core::svn_config_get_bool($conf_t,
> + $SVN::_Core::SVN_CONFIG_SECTION_AUTH,
> + $SVN::_Core::SVN_CONFIG_OPTION_STORE_PASSWORDS,
> + 1) == 0) {
> + SVN::_Core::svn_auth_set_parameter($baton,
> + $SVN::_Core::SVN_AUTH_PARAM_DONT_STORE_PASSWORDS,
> + bless (\$dont_store_passwords, "_p_void"));
> + }
> + if (SVN::_Core::svn_config_get_bool($conf_t,
> + $SVN::_Core::SVN_CONFIG_SECTION_AUTH,
> + $SVN::_Core::SVN_CONFIG_OPTION_STORE_AUTH_CREDS,
> + 1) == 0) {
> + $Git::SVN::Prompt::_no_auth_cache = 1;
> + }
> + }
> my $self = SVN::Ra->new(url => $url, auth => $baton,
> config => $config,
> pool => SVN::Pool->new,
--
Eric Wong
^ permalink raw reply
* Re: [PATCH] Make git-clean a builtin
From: Linus Torvalds @ 2007-10-07 1:31 UTC (permalink / raw)
To: Shawn Bohrer; +Cc: git, frank, gitster
In-Reply-To: <1191719841666-git-send-email-shawn.bohrer@gmail.com>
On Sat, 6 Oct 2007, Shawn Bohrer wrote:
>
> This replaces git-clean.sh with builtin-clean.c, and moves git-clean.sh to the
> examples.
This looks better, but I think you'd be even better off actually using the
"read_directory()" interface directly, instead of exec'ing off "git
ls-files" and parsing the line output.
I also would still worry a bit about 'chdir(x)' and 'chdir("..")', because
quite frankly, they are *not* mirrors of each other (think symlinks, but
also error behaviour due to directories that might be non-executable).
Now, admittedly, if a directory isn't executable, I can imagine other git
things having problems (anybody want to test?), but that whole pattern is
just very fragile and not very reliable.
Linus
^ permalink raw reply
* Re: git-push [--all] and tags
From: Linus Torvalds @ 2007-10-07 1:26 UTC (permalink / raw)
To: Sam Vilain; +Cc: martin f krafft, git discussion list
In-Reply-To: <47083035.7070904@vilain.net>
On Sun, 7 Oct 2007, Sam Vilain wrote:
>
> Think is, pushing tags is considered to be something that needs to be
> done very explicitly; the convention, after all, is that published tags
> are forever. Whereas branches change all the time. So, pushing tags
> should require an extra flag.
Well, I agrewe with Martin, and I think you missed his point.
Sure, pushing tags should require you to say so explicitly, but right now
you cannot (for example) push *both* the common branches *and* tags
easily.
You can do
git push
to push common branches, and you can do
git push --tags
to push tags, but there's no way to say "push both the common branches
_and_ all tags".
(And the same is true of "--all" and "--tags").
So I do think Martin is right. I think we should:
- make sure that "--all --tags" DTRT, namely send all branches and all
tags. Right now they are actually very similar things (one works on
"refs/heads", while the other works on "refs/tags"), but they are
implemented using *totally* different logic, and they don't work
together.
- I think it would be a good idea to implement a "--shared" flag, which
is the current default behavior when there are no command line flags,
and there are no listed branches to push in the config file. That would
allow doing "git push --shared --tags" to update shared branches _and_
tags, but it would probably *also* make it easier to explain the
default behaviour of "git push", by making the behavior more explicit.
(IOW, you can introduce the notion of "--shared updates all branches
that exists both locally and remotely under the same names" separately
early on as a command line option, and then later just explain that if
there is nothing else telling git what to push, it will default to
that shared branch behavior)
Hmm? I do agree with "git push" as it now stands has a lot of warts,
although I don't really agree with the people who want to change the
default behavior.
Linus
^ permalink raw reply
* Re: [PATCH] git-svn: respect Subversion's [auth] section configuration values
From: Sam Vilain @ 2007-10-07 1:24 UTC (permalink / raw)
To: Eygene Ryabinkin; +Cc: git, Eric Wong
In-Reply-To: <20071006185719.GA3943@void.codelabs.ru>
Eygene Ryabinkin wrote:
> Parameters 'store-passwords' and 'store-auth-creds' from Subversion's
> configuration (~/.subversion/config) were not respected. This was
> fixed: the default values for these parameters are set to 'yes' to
> follow Subversion behaviour.
>
I saw this in the svn api before. It really is a strange API, requiring
the user to get things like this right.
> Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>
> ---
> git-svn.perl | 23 +++++++++++++++++++++++
> 1 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index 484b057..f7ef421 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -3051,6 +3051,29 @@ sub new {
> ]);
> my $config = SVN::Core::config_get_config($config_dir);
> $RA = undef;
> + my $dont_store_passwords = 1;
> + my $conf_t = ${$config}{'config'};
> + {
> + # The usage of $SVN::_Core::SVN_CONFIG_* variables
> + # produces warnings that variables are used only once.
> + # I had not found the better way to shut them up, so
> + # warnings are disabled in this block.
> + no warnings;
>
You can use no warnings 'once';
> + if (SVN::_Core::svn_config_get_bool($conf_t,
> + $SVN::_Core::SVN_CONFIG_SECTION_AUTH,
> + $SVN::_Core::SVN_CONFIG_OPTION_STORE_PASSWORDS,
> + 1) == 0) {
> + SVN::_Core::svn_auth_set_parameter($baton,
> + $SVN::_Core::SVN_AUTH_PARAM_DONT_STORE_PASSWORDS,
> + bless (\$dont_store_passwords, "_p_void"));
> + }
> + if (SVN::_Core::svn_config_get_bool($conf_t,
> + $SVN::_Core::SVN_CONFIG_SECTION_AUTH,
> + $SVN::_Core::SVN_CONFIG_OPTION_STORE_AUTH_CREDS,
> + 1) == 0) {
> + $Git::SVN::Prompt::_no_auth_cache = 1;
> + }
> + }
> my $self = SVN::Ra->new(url => $url, auth => $baton,
> config => $config,
> pool => SVN::Pool->new,
>
^ permalink raw reply
* [PATCH] Make git-clean a builtin
From: Shawn Bohrer @ 2007-10-07 1:17 UTC (permalink / raw)
To: git; +Cc: frank, gitster, Shawn Bohrer
This replaces git-clean.sh with builtin-clean.c, and moves git-clean.sh to the
examples.
Signed-off-by: Shawn Bohrer <shawn.bohrer@gmail.com>
---
Reworked patch with fixes to Frank's suggestions.
Makefile | 3 +-
builtin-clean.c | 177 +++++++++++++++++++++++++
builtin.h | 1 +
git-clean.sh => contrib/examples/git-clean.sh | 0
git.c | 1 +
5 files changed, 181 insertions(+), 1 deletions(-)
create mode 100644 builtin-clean.c
rename git-clean.sh => contrib/examples/git-clean.sh (100%)
diff --git a/Makefile b/Makefile
index 8db4dbe..2b3b8fb 100644
--- a/Makefile
+++ b/Makefile
@@ -206,7 +206,7 @@ BASIC_LDFLAGS =
SCRIPT_SH = \
git-bisect.sh git-checkout.sh \
- git-clean.sh git-clone.sh git-commit.sh \
+ git-clone.sh git-commit.sh \
git-fetch.sh \
git-ls-remote.sh \
git-merge-one-file.sh git-mergetool.sh git-parse-remote.sh \
@@ -327,6 +327,7 @@ BUILTIN_OBJS = \
builtin-check-attr.o \
builtin-checkout-index.o \
builtin-check-ref-format.o \
+ builtin-clean.o \
builtin-commit-tree.o \
builtin-count-objects.o \
builtin-describe.o \
diff --git a/builtin-clean.c b/builtin-clean.c
new file mode 100644
index 0000000..4890c2d
--- /dev/null
+++ b/builtin-clean.c
@@ -0,0 +1,177 @@
+/*
+ * "git clean" builtin command
+ *
+ * Copyright (C) 2007 Shawn Bohrer
+ *
+ * Based on git-clean.sh by Pavel Roskin
+ */
+
+#include "builtin.h"
+#include "cache.h"
+#include "run-command.h"
+
+static int disabled = 0;
+static int show_only = 0;
+static int remove_directories = 0;
+static int quiet = 0;
+static int ignored = 0;
+static int ignored_only = 0;
+
+static const char builtin_clean_usage[] =
+"git-clean [-d] [-f] [-n] [-q] [-x | -X] [--] <paths>...";
+
+static int git_clean_config(const char *var, const char *value)
+{
+ if (!strcmp(var, "clean.requireforce")) {
+ disabled = git_config_bool(var, value);
+ }
+ return 0;
+}
+
+static int remove_directory(const char *path)
+{
+ DIR *d;
+ struct dirent *dir;
+ d = opendir(path);
+ if (d) {
+ chdir(path);
+ while ((dir = readdir(d)) != NULL) {
+ if(strcmp( dir->d_name, ".") == 0 ||
+ strcmp( dir->d_name, ".." ) == 0 )
+ continue;
+ if (dir->d_type == DT_DIR)
+ remove_directory(dir->d_name);
+ else
+ unlink(dir->d_name);
+ }
+ chdir("..");
+ }
+ closedir(d);
+ return rmdir(path);
+}
+
+int cmd_clean(int argc, const char **argv, const char *prefix)
+{
+ int i;
+ int j;
+ struct child_process cmd;
+ const char **argv_ls_files;
+ char *buf = NULL;
+ char path[1024];
+ FILE *cmd_fout;
+
+ git_config(git_clean_config);
+
+ 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;
+ disabled = 0;
+ continue;
+ }
+ if (!strcmp(arg, "-f")) {
+ disabled = 0;
+ continue;
+ }
+ if (!strcmp(arg, "-d")) {
+ remove_directories = 1;
+ continue;
+ }
+ if (!strcmp(arg, "-q")) {
+ quiet = 1;
+ continue;
+ }
+ if (!strcmp(arg, "-x")) {
+ ignored = 1;
+ continue;
+ }
+ if (!strcmp(arg, "-X")) {
+ ignored_only = 1;
+ continue;
+ }
+ usage(builtin_clean_usage);
+ }
+
+ if (ignored && ignored_only)
+ usage(builtin_clean_usage);
+
+ if (disabled) {
+ die("clean.requireForce set and -n or -f not given; refusing to clean");
+ }
+
+ /* Paths (argc - i) + 8 (Possible arguments)*/
+ argv_ls_files = xmalloc((argc - i + 8) * sizeof(const char *));
+ argv_ls_files[0] = "ls-files";
+ argv_ls_files[1] = "--others";
+ argv_ls_files[2] = "--directory";
+ j = 3;
+ if (!ignored) {
+ argv_ls_files[j++] = "--exclude-per-directory=.gitignore";
+ if (ignored_only)
+ argv_ls_files[j++] = "--ignored";
+ if (!access(git_path("info/exclude"), F_OK)) {
+ char *exclude_path = git_path("info/exclude");
+ int len = strlen(exclude_path);
+ buf = (char*)malloc(len+16);
+ sprintf(buf, "--exclude-from=%s", exclude_path);
+ argv_ls_files[j++] = buf;
+ }
+ }
+ argv_ls_files[j++] = "--";
+ /* Add remaining paths passed in as arguments */
+ if (argc - i)
+ memcpy(argv_ls_files + j++, argv + i, (argc - i) * sizeof(const char *));
+ argv_ls_files[j + argc - i] = NULL;
+
+ memset(&cmd, 0, sizeof(cmd));
+ cmd.argv = argv_ls_files;
+ cmd.git_cmd = 1;
+ cmd.out = -1;
+ if (start_command(&cmd))
+ die("Could not run sub-command: git ls-files");
+
+ cmd_fout = fdopen(cmd.out, "r");
+ while (fgets(path, sizeof(path), cmd_fout) != NULL) {
+ struct stat st;
+ char *p;
+ p = strrchr(path, '\n');
+ if ( p != NULL )
+ *p = '\0';
+ if (!lstat(path, &st) && (S_ISDIR(st.st_mode))) {
+ if (show_only && remove_directories) {
+ printf("Would remove %s\n", path);
+ } else if (quiet && remove_directories) {
+ remove_directory(path);
+ } else if (remove_directories) {
+ printf("Removing %s\n", path);
+ remove_directory(path);
+ } else if (show_only) {
+ printf("Would not remove %s\n", path);
+ } else {
+ printf("Not removing %s\n", path);
+ }
+ } else {
+ if (show_only) {
+ printf("Would remove %s\n", path);
+ continue;
+ } else if (!quiet) {
+ printf("Removing %s\n", path);
+ }
+ unlink(path);
+ }
+ }
+
+ fclose(cmd_fout);
+ finish_command(&cmd);
+ if (buf != NULL)
+ free(buf);
+ free(argv_ls_files);
+ return 0;
+}
diff --git a/builtin.h b/builtin.h
index d6f2c76..8c112f3 100644
--- a/builtin.h
+++ b/builtin.h
@@ -23,6 +23,7 @@ extern int cmd_check_attr(int argc, const char **argv, const char *prefix);
extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);
extern int cmd_cherry(int argc, const char **argv, const char *prefix);
extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);
+extern int cmd_clean(int argc, const char **argv, const char *prefix);
extern int cmd_commit_tree(int argc, const char **argv, const char *prefix);
extern int cmd_count_objects(int argc, const char **argv, const char *prefix);
extern int cmd_describe(int argc, const char **argv, const char *prefix);
diff --git a/git-clean.sh b/contrib/examples/git-clean.sh
similarity index 100%
rename from git-clean.sh
rename to contrib/examples/git-clean.sh
diff --git a/git.c b/git.c
index 9eaca1d..cda6344 100644
--- a/git.c
+++ b/git.c
@@ -320,6 +320,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
{ "cherry", cmd_cherry, RUN_SETUP },
{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
+ { "clean", cmd_clean, RUN_SETUP },
{ "commit-tree", cmd_commit_tree, RUN_SETUP },
{ "config", cmd_config },
{ "count-objects", cmd_count_objects, RUN_SETUP },
--
1.5.3.GIT
^ permalink raw reply related
* Re: [PATCH] Make git-clean a builtin
From: Shawn Bohrer @ 2007-10-07 1:13 UTC (permalink / raw)
To: Frank Lichtenheld; +Cc: git, gitster
In-Reply-To: <20071006215253.GX31659@planck.djpig.de>
On Sat, Oct 06, 2007 at 11:52:53PM +0200, Frank Lichtenheld wrote:
> On Sat, Oct 06, 2007 at 03:54:06PM -0500, Shawn Bohrer wrote:
> > +static int remove_directory(const char *path)
> > +{
> > + DIR *d;
> > + struct dirent *dir;
> > + d = opendir(path);
> > + if (d) {
> > + chdir(path);
> > + while ((dir = readdir(d)) != NULL) {
> > + if(strcmp( dir->d_name, ".") == 0 ||
> > + strcmp( dir->d_name, ".." ) == 0 )
> > + continue;
> > + if (dir->d_type == DT_DIR)
> > + remove_directory(dir->d_name);
> > + else
> > + unlink(dir->d_name);
> > + }
> > + }
> > + closedir(d);
> > + chdir("..");
> > + return rmdir(path);
> > +}
>
> The unconditional chdir(..) after the conditional chdir(path) seems like
> asking for trouble to me...
Agreed, I'm not sure what I was thinking there.
> > + while (fgets(path, sizeof(path), cmd_fout) != NULL) {
> > + struct stat st;
> > + char *p;
> > + p = strrchr(path, '\n');
> > + if ( p != NULL )
> > + *p = '\0';
>
> What happens in case p == NULL? It simply tries to remove the partial
> path?
If p == NULL then the path didn't have a EOL character. This shouldn't
ever really happen since fgets() leaves the EOL character as part of the
string, and it is processing the output of git-ls-files which will
provide one path per line. If it does happen for some reason then
either we will happily remove the file/directory, or if the path is
garbage then we will simply fail to remove anything.
> > + fclose(cmd_fout);
> > + finish_command(&cmd);
> > + if (!ignored && !access(git_path("info/exclude"), F_OK))
> > + free(buf);
>
> There is a race condition here of the value of access() changes between
> the two calls. Not one likely to trigger but it should be easy to avoid
> alltogether.
Yes, though unlikely I agree this wasn't very smart in the first place
so I fixed it and will send reworked patch soon.
--
Shawn
^ permalink raw reply
* Re: git-push [--all] and tags
From: Sam Vilain @ 2007-10-07 1:02 UTC (permalink / raw)
To: martin f krafft; +Cc: git discussion list
In-Reply-To: <20071006160506.GA28238@lapse.madduck.net>
martin f krafft wrote:
> Hello people,
>
> `git-push --all --tags` does not work because git-push ends up
> calling git-send-pack --all refs/tags/*, which the latter does not
> deal with.
>
> Looking at the code, it seems that previously, --all would push
> everything, not just refs/heads/*. What's the reason that this was
> changed? Why aren't tags considered part of --all?
>
> If I wanted to fix this, so that --all pushes heads and --all --tags
> pushes heads and tags, I could do so in two ways:
>
> 1. instead of --all, pass refs/heads/* to git-send-pack
> 2. add --tags to git-send-pack
>
> which of these two would you prefer and why?
>
Think is, pushing tags is considered to be something that needs to be
done very explicitly; the convention, after all, is that published tags
are forever. Whereas branches change all the time. So, pushing tags
should require an extra flag.
Sam.
^ permalink raw reply
* Re: [PATCH] setup/rev-parse: allow HEAD to be spelled 'head'
From: Sam Vilain @ 2007-10-06 22:23 UTC (permalink / raw)
To: Alex Riesen; +Cc: git, Johannes Schindelin
In-Reply-To: <20071005065139.GA2623@steel.home>
Alex Riesen wrote:
> Sam Vilain, Fri, Oct 05, 2007 05:09:10 +0200:
>> If the repository got mangled by FAT capitalization rules, then a ref
>> such as "HEAD" will become "head" once it is back on a non-FAT FS.
>> Check for this condition in resolve_refs and in the setup code.
>>
>> Suggested-by: Francois Marier <francois@debian.org>
>> Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>
>> ---
>> This should probably help people putting their git repos on
>> FAT USB sticks.
>
> Can the people just mount FAT partitions with shortname=mixed?
Of course, that is probably a solution to the problem, whereas my patch
is a workaround.
Now, I realise that this might open a can of worms ... would we also
want to go looking for files called "pack-ab~1.pac" ? Almost certainly
not - but this solves the most immediate problem experienced by people
putting their git repositories onto FAT filesystems mounted with the
default options, which will say "FATAL: not a git repository" otherwise.
Sam.
^ permalink raw reply
* Re: [msysGit] Re: Git Inno Setup based installer
From: Steffen Prohaska @ 2007-10-06 22:15 UTC (permalink / raw)
To: kirillathome; +Cc: msysGit, Git Mailing List, Shawn O. Pearce
In-Reply-To: <1191682524.589150.297160@22g2000hsm.googlegroups.com>
Dear Kirill,
Thanks for your feedback. I very much appreciate any comments on the
installer. They all help to improve our work.
On Oct 6, 2007, at 4:55 PM, Kirill wrote:
> Great! Thanks to you I finally got git-1.5.3 that works on Windows (at
> least locally)! Appreciated!
good to hear.
> Now, didn't you ask for a feedback? :)
>
> I. Each time I start git-gui it tells me that:
> Git version cannot be determined.
> C:/wingit/bin/git.exe claims it is version '1.5.3.mingw.1.99.ga821'.
> Git Gui requires at least Git 1.5.0 or later.
> Assume '1.5.3.mingw.1.99.ga821' is version 1.5.0?
>
> Before I try to hunt it down, is it an easy fix?
It is easy to fixed and I already have a patch in the pipeline.
I know it's a bit annoying but it's not really a critical issue.
Therefore I'll not create a new installer right away.
> II. I noticed that in the latest version of Git Gui then Branch menu
> does not have items to quickly switch to another branch (basically
> items, named after branches). Is it very hard to get them back?
>
> I did not report it earlier because all my previous attempts to get
> fully working version failed.
Note, I'm currently focusing on the Windows installer, while git-gui
is a project of it's own. Git gui is not specific to Windows but a
cross platform tool that runs on Unix as well. git-gui is part of the
official git repository. I just take what git-gui offers and pack it
into the installer. Most of the real work was done by Shawn O. Pearce.
The right platform for discussing git-gui specific issues is the
official git mailing list (not the msysgit list). I added the list
in CC. I can't comment on this issue in detail because I'm not using
Git Gui that frequently. Maybe someone else from the git mailing list
has a comment?
> III. Also, I was pleasantly surprised that git-gui is localized to
> Russian (probably among many other languages). However, the above
> message is missing from localization. So on Russian system I got an
> empty message, asking Yes or No. Is it easy to fix too? If you want
> real Russian there, here you are:
> Не могу определить версию Git.
> Версия C:/wingit/bin/git.exe: '1.5.3.mingw.1.99.ga821'.
> Git Gui требует как минимум Git 1.5.0.
> Считать '1.5.3.mingw.1.99.ga821' версией 1.5.0?
I know about the localization efforts but I did not closely follow
them. All the cedits go to people on the git mailing list who worked
on the localization.
Maybe someone could pick up your suggestion and prepare a patch for
git-gui? Personally, I don't know how to do this.
Steffen
> On Oct 5, 3:30 pm, Steffen wrote:
>
>> I also created an installer, which you can find at
>>
>> http://www.zib.de/prohaska/2007/git/Git-1.5.3-preview20071005.exe
>>
>> The installer contains the branches devel merged with
>> steffen/shawn-git-gui.
^ permalink raw reply
* Re: [PATCH] Make git-clean a builtin
From: Frank Lichtenheld @ 2007-10-06 21:52 UTC (permalink / raw)
To: Shawn Bohrer; +Cc: git, gitster
In-Reply-To: <11917040461528-git-send-email-shawn.bohrer@gmail.com>
On Sat, Oct 06, 2007 at 03:54:06PM -0500, Shawn Bohrer wrote:
> +static int remove_directory(const char *path)
> +{
> + DIR *d;
> + struct dirent *dir;
> + d = opendir(path);
> + if (d) {
> + chdir(path);
> + while ((dir = readdir(d)) != NULL) {
> + if(strcmp( dir->d_name, ".") == 0 ||
> + strcmp( dir->d_name, ".." ) == 0 )
> + continue;
> + if (dir->d_type == DT_DIR)
> + remove_directory(dir->d_name);
> + else
> + unlink(dir->d_name);
> + }
> + }
> + closedir(d);
> + chdir("..");
> + return rmdir(path);
> +}
The unconditional chdir(..) after the conditional chdir(path) seems like
asking for trouble to me...
> + while (fgets(path, sizeof(path), cmd_fout) != NULL) {
> + struct stat st;
> + char *p;
> + p = strrchr(path, '\n');
> + if ( p != NULL )
> + *p = '\0';
What happens in case p == NULL? It simply tries to remove the partial
path?
> + fclose(cmd_fout);
> + finish_command(&cmd);
> + if (!ignored && !access(git_path("info/exclude"), F_OK))
> + free(buf);
There is a race condition here of the value of access() changes between
the two calls. Not one likely to trigger but it should be easy to avoid
alltogether.
> + free(argv_ls_files);
> + return 0;
> +}
Gruesse,
--
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/
^ 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