Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Makefile: Use libc strlcpy on OSX
From: Benjamin Kramer @ 2009-01-24 18:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfxj8o8fq.fsf@gitster.siamese.dyndns.org>

On Sat, Jan 24, 2009 at 19:39, Junio C Hamano <gitster@pobox.com> wrote:
> How does your patch to Makefile refrain from kicking in for people with
> OSX older than 10.2, so that their builds are not broken?

I'm not even sure anyone uses <= 10.1 nowadays. But of course you're right,
I'll write a better patch.

-- Benjamin

^ permalink raw reply

* Re: problems with http://git-scm.com/
From: Teemu Likonen @ 2009-01-24 18:44 UTC (permalink / raw)
  To: Oliver Kullmann; +Cc: git, Adam Cooke, Scott Chacon
In-Reply-To: <20090124173756.GU6683@cs-wsok.swansea.ac.uk>

[Adding some CCs.]


Oliver Kullmann (2009-01-24 17:37 +0000) wrote:

> since a few docs my (somewhat rusty) Konqueror can't load
> http://git-scm.com/
> anymore.
>
> Now checking at
> http://validator.w3.org/
> we get one error:
>
>  Line 174, Column 14: there is no attribute "clear".
>     <br clear="all"/>
>
> And that shouldn't be (since http://git-scm.com/ claims to be strict
> xhtml). Don't know whether this is the problem, but that attribute
> clearly should be removed anyway.

It seems that the attribute has already been changed to
style="clear:both;" in the web site's git repository[1]. It's the commit
cea494929e (2009-01-08). But www.git-scm.com is still serving an old
version.

---------------
 1. git://github.com/schacon/gitscm.git

^ permalink raw reply

* Re: read-only working copies using links
From: Sverre Rabbelier @ 2009-01-24 18:43 UTC (permalink / raw)
  To: Chad Dombrova; +Cc: Tim 'Mithro' Ansell, git
In-Reply-To: <AE9781AD-0B63-4AEE-9B62-20F9720B5DC4@gmail.com>

On Sat, Jan 24, 2009 at 19:39, Chad Dombrova <chadrik@gmail.com> wrote:
> what's the case against it, other than the obvious, that it will take more
> work?

Good question, I think it was mostly that, someone has to implement it
(possibly as part of packv4). Backwards compatibility is of course
always an concern, but I'm not too familiar with the subject, perhaps
other people on the list (or even those were at the gittogether) can
comment?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: read-only working copies using links
From: Chad Dombrova @ 2009-01-24 18:39 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Tim 'Mithro' Ansell, git
In-Reply-To: <bd6139dc0901240302q6b3c9a13p56fe39a976882133@mail.gmail.com>

>
> I think Tim Ansell (cced) was talking about this at the gittogether
> (storing the metadata seperately), as it would benefit sparse/narrow
> checkout, another advantage supporting his case?
>

what's the case against it, other than the obvious, that it will take  
more work?


-chad

^ permalink raw reply

* Re: [PATCH] Makefile: Use libc strlcpy on OSX
From: Junio C Hamano @ 2009-01-24 18:39 UTC (permalink / raw)
  To: Benjamin Kramer; +Cc: git
In-Reply-To: <7f978c810901240741k201f954dx1c0470186094ae24@mail.gmail.com>

Benjamin Kramer <benny.kra@googlemail.com> writes:

> OSX supports strlcpy(3) since 10.2 so we don't need
> to use our own.

How does your patch to Makefile refrain from kicking in for people with
OSX older than 10.2, so that their builds are not broken?

>
> Signed-off-by: Benjamin Kramer <benny.kra@gmail.com>
> ---
>  Makefile |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index b4d9cb4..4c9aab5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -643,7 +643,6 @@ ifeq ($(uname_S),Darwin)
>  	ifneq ($(shell expr "$(uname_R)" : '9\.'),2)
>  		OLD_ICONV = UnfortunatelyYes
>  	endif
> -	NO_STRLCPY = YesPlease
>  	NO_MEMMEM = YesPlease
>  	THREADED_DELTA_SEARCH = YesPlease
>  endif

^ permalink raw reply

* Re: [PATCH 1/2] handle color.ui at a central place
From: Junio C Hamano @ 2009-01-24 18:36 UTC (permalink / raw)
  To: markus.heidelberg; +Cc: Jeff King, René Scharfe, git
In-Reply-To: <200901241228.33690.markus.heidelberg@web.de>

Markus Heidelberg <markus.heidelberg@web.de> writes:

> I looked at the code and found this in git_format_config():
>
> 	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
> 		return 0;
>
> Which of course didn't handle color.ui, but that wasn't necessary before
> the central color.ui handling from my patch.

Centralized handling is never a goal in itself.  The goal should be to
make it easier for various codepaths to use color settings correctly,
without having to have many special case workarounds.  Centralized
handling, if designed right, could be a good way to achieve that goal.

> So with the following diff it works:
>
> -	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
> +	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")
> +				       || !strcmp(var, "color.ui")) {

Why should format-patch need to even worry about protecting itself from
"color.ui" to begin with?

If your patch is making color handling saner, I would expect that
format-patch can *lose* the existing "ignore diff.color or color.diff"
workaround as a result of that.  If you need to add even *more* workaround
code like that, there's something wrong, don't you think?

> format-patch is perhaps the only place where the commit has broken
> things, because I didn't find other places,...

You did not find the breakage in format-patch either to begin with; so
your not finding does not give us much confidence that there is no other
breakage, does it?

Grumble...

^ permalink raw reply

* Re: [PATCH] mergetool merge/skip/abort
From: Caleb Cushing @ 2009-01-24 18:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Charles Bailey, git
In-Reply-To: <7vwscmue5z.fsf@gitster.siamese.dyndns.org>

> I do not use mergetool myself so I generally do not pay attention to
>  patches on this tool, but I would want to pick up ones that people
>  involved in mergetool discussion can agree to be good patches.

yeah I can see that.

>  There are a few mergetool updates in flight from various authors.  How
>  does your submission compare with others' in both form/presentation and
>  clarity of logic (remember, I am not keeping track)?

to be honest, a quick search of the past 2 months of patches didn't
show me any patches that do the same thing as mine, so I'm not sure
that comparing one feature to a different feature is good. I did try
to remain consistent and even improve consistency with existing UI,
and use the same/similar logic to existing.  I'm not keeping track
either, just fixing my own problem.
-- 
Caleb Cushing

http://xenoterracide.blogspot.com

^ permalink raw reply

* problems with http://git-scm.com/
From: Oliver Kullmann @ 2009-01-24 17:37 UTC (permalink / raw)
  To: git

Hi,

since a few docs my (somewhat rusty) Konqueror can't load
http://git-scm.com/
anymore.

Now checking at
http://validator.w3.org/
we get one error:

 Line 174, Column 14: there is no attribute "clear".
    <br clear="all"/>

And that shouldn't be (since http://git-scm.com/ claims to
be strict xhtml).
Don't know whether this is the problem, but that attribute
clearly should be removed anyway.

Oliver

^ permalink raw reply

* [PATCH] git-svn: Don't fail `--help' if git directory not found.
From: Allan Caffee @ 2009-01-24 17:44 UTC (permalink / raw)
  To: git

When called with the `--help' or `--version' from outside of a working tree
git-svn would fail with

	fatal: Not a git repository
	rev-parse --show-cdup: command returned error: 128

This happens because git-svn would check that it was running from the top of a
working directory before parsing options.  Fix this by parsing options first.

Signed-off-by: Allan Caffee <allan.caffee@gmail.com>
---
 git-svn.perl |   38 +++++++++++++++++++-------------------
 1 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index d4cb538..875a05b 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -216,6 +216,25 @@ for (my $i = 0; $i < @ARGV; $i++) {
 	}
 };
 
+my %opts = %{$cmd{$cmd}->[2]} if (defined $cmd);
+
+read_repo_config(\%opts);
+if ($cmd && ($cmd eq 'log' || $cmd eq 'blame')) {
+	Getopt::Long::Configure('pass_through');
+}
+my $rv = GetOptions(%opts, 'help|H|h' => \$_help, 'version|V' => \$_version,
+                    'minimize-connections' => \$Git::SVN::Migration::_minimize,
+                    'id|i=s' => \$Git::SVN::default_ref_id,
+                    'svn-remote|remote|R=s' => sub {
+                       $Git::SVN::no_reuse_existing = 1;
+                       $Git::SVN::default_repo_id = $_[1] });
+exit 1 if (!$rv && $cmd && $cmd ne 'log');
+
+usage(0) if $_help;
+version() if $_version;
+usage(1) unless defined $cmd;
+load_authors() if $_authors;
+
 # make sure we're always running at the top-level working directory
 unless ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {
 	unless (-d $ENV{GIT_DIR}) {
@@ -241,25 +260,6 @@ unless ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {
 	$_repository = Git->repository(Repository => $ENV{GIT_DIR});
 }
 
-my %opts = %{$cmd{$cmd}->[2]} if (defined $cmd);
-
-read_repo_config(\%opts);
-if ($cmd && ($cmd eq 'log' || $cmd eq 'blame')) {
-	Getopt::Long::Configure('pass_through');
-}
-my $rv = GetOptions(%opts, 'help|H|h' => \$_help, 'version|V' => \$_version,
-                    'minimize-connections' => \$Git::SVN::Migration::_minimize,
-                    'id|i=s' => \$Git::SVN::default_ref_id,
-                    'svn-remote|remote|R=s' => sub {
-                       $Git::SVN::no_reuse_existing = 1;
-                       $Git::SVN::default_repo_id = $_[1] });
-exit 1 if (!$rv && $cmd && $cmd ne 'log');
-
-usage(0) if $_help;
-version() if $_version;
-usage(1) unless defined $cmd;
-load_authors() if $_authors;
-
 unless ($cmd =~ /^(?:clone|init|multi-init|commit-diff)$/) {
 	Git::SVN::Migration::migration_check();
 }
-- 
1.5.4.3

^ permalink raw reply related

* diff settings
From: Keith Cascio @ 2009-01-24 17:38 UTC (permalink / raw)
  To: git

How do I configure my local git so that diff always obeys a particular option, 
e.g. "-w", without needing to type it on the command line each time?

Thanks,
Keith

^ permalink raw reply

* Re: [PATCH] Use time_t for timestamps returned by approxidate()  instead of unsigned
From: Johannes Schindelin @ 2009-01-24 17:32 UTC (permalink / raw)
  To: Tim Henigan; +Cc: git, pasky, gitster
In-Reply-To: <32c343770901240914n3fc76fe0vbe844e50cbc0bbf@mail.gmail.com>

Hi,

On Sat, 24 Jan 2009, Tim Henigan wrote:

> On Sat, Jan 24, 2009 at 11:58 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>
> >> Should I update the GitWiki page to remove this Janitor task or do 
> >> you keep it as a test to see if people are properly searching the 
> >> mail archives?
> >
> > Yes, please!
> 
> I did some more digging and found this last comment on the subject by 
> Linus: https://kerneltrap.org/mailarchive/git/2008/11/6/4014344
> 
> Given this, should the janitor task simply be deleted (since using 
> unsigned longs are safe until year 2038 is considered) or should it be 
> updated to change all timestamps to 64-bit values?
> 
> Also, there are still ~37 references to time_t in the code (e.g. the 
> index_state struct in cache.h).  Should these be phased out in favor of 
> unsigned longs (or perhaps 64-bit types)?

Correct me if I am wrong: there are two different uses of times.  One to 
output dates (which is where we use unsigned long), and one where we 
compare dates in file stats (where we use time_t).

I haven't looked at the code, though, so this might be incorrect.

If it is correct, though, I think that we should stay with time_t for the 
cases where we have to compare to time_t anyway (as the stat() calls give 
us the time in that datatype).

BTW the Janitor task was added by Pasky, so he should say something.  
Pasky?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Use time_t for timestamps returned by approxidate()  instead of unsigned
From: Tim Henigan @ 2009-01-24 17:14 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0901241757140.13232@racer>

On Sat, Jan 24, 2009 at 11:58 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>> Should I update the GitWiki page to remove this Janitor task or do you
>> keep it as a test to see if people are properly searching the mail
>> archives?
>
> Yes, please!

I did some more digging and found this last comment on the subject by Linus:
https://kerneltrap.org/mailarchive/git/2008/11/6/4014344

Given this, should the janitor task simply be deleted (since using
unsigned longs are safe until year 2038 is considered) or should it be
updated to change all timestamps to 64-bit values?

Also, there are still ~37 references to time_t in the code (e.g. the
index_state struct in cache.h).  Should these be phased out in favor
of unsigned longs (or perhaps 64-bit types)?

Perhaps the Janitor task should be changed to do this so that all the
code handles timestamps consistently?  (btw, I will volunteer to do
this if desired).

Thanks,
Tim

^ permalink raw reply

* Re: PATCH] http-push: refactor request url creation
From: Johannes Schindelin @ 2009-01-24 17:03 UTC (permalink / raw)
  To: Ray Chuan; +Cc: git
In-Reply-To: <be6fef0d0901240816r25e6a9c9q2d437d413a524362@mail.gmail.com>

Hi,

On Sun, 25 Jan 2009, Ray Chuan wrote:

> Currently, functions that deal with objects on the remote repository
> have to allocate and do strcpys to generate the URL.
> 
> This patch saves them this trouble, by providing a function that
> returns a URL: either the object's 2-digit hex directory (eg.
> /objects/a1/) or the complete object location (eg. /objects/a1/b2).
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Make that an Acked-by: okay?

> diff --git a/http-push.c b/http-push.c
> index cb5bf95..715954e 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -177,6 +177,16 @@ struct remote_ls_ctx
>  	struct remote_ls_ctx *parent;
>  };
> 
> +static char *get_remote_object_url(const char *url, const char *hex,
> int only_two_digit_postfix) {

It is actually a prefix.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Use time_t for timestamps returned by approxidate()  instead of unsigned
From: Johannes Schindelin @ 2009-01-24 16:58 UTC (permalink / raw)
  To: Tim Henigan; +Cc: git, gitster
In-Reply-To: <32c343770901240813k2eeb19b0q65b533f829cb44d4@mail.gmail.com>

Hi,

On Sat, 24 Jan 2009, Tim Henigan wrote:

> On Sat, Jan 24, 2009 at 1:27 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Thu, 22 Jan 2009, Tim Henigan wrote:
> >
> > > Use time_t for timestamps returned by approxidate() instead of 
> > > unsigned long.  All references to approxidate were checked as well 
> > > as references to OPT_DATE.
> >
> > Hmm.  I vaguely remember Linus mentioning recently that unsigned long 
> > is the appropriate data type for the Unix Epoch...
> 
> You are correct. I just found a post on this list where Linus 
> specifically rejected a similar patch from another contributor.  A quote 
> from https://kerneltrap.org/mailarchive/git/2008/11/6/4014124:
> 
>     " "time_t" is one of those totally broken unix types. The standards 
>     say that it's an "arithmetic" type, but leaves it open to be just 
>     about anything. Traditionally, it's a signed integer (bad), and in 
>     theory it could even be a floating point value, I think.
> 
>     And in _all_ such cases, it's actually better to cast it to 
>     "unsigned long" than keep time in a system-dependent format that is 
>     most likely either _already_ "unsigned long", or alternatively 
>     broken."

Thanks.  I should have searched that post myself, but I was too tired...

> Should I update the GitWiki page to remove this Janitor task or do you
> keep it as a test to see if people are properly searching the mail
> archives?

Yes, please!

> Sorry to waste your time on the patch review.

Sorry that we wasted your time by letting that page go stale...

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Use time_t for timestamps returned by approxidate()  instead of unsigned
From: Wincent Colaiuta @ 2009-01-24 16:37 UTC (permalink / raw)
  To: Tim Henigan; +Cc: Johannes Schindelin, git, gitster
In-Reply-To: <32c343770901240813k2eeb19b0q65b533f829cb44d4@mail.gmail.com>

El 24/1/2009, a las 17:13, Tim Henigan escribió:

> Should I update the GitWiki page to remove this Janitor task or do you
> keep it as a test to see if people are properly searching the mail
> archives?

I don't know of too many projects which intentionally publish  
misleading information to "test" would-be contributors. I think  
editing the wiki would be a good idea.

Cheers,
Wincent

^ permalink raw reply

* PATCH] http-push: refactor request url creation
From: Ray Chuan @ 2009-01-24 16:16 UTC (permalink / raw)
  To: git, Johannes Schindelin

Currently, functions that deal with objects on the remote repository
have to allocate and do strcpys to generate the URL.

This patch saves them this trouble, by providing a function that
returns a URL: either the object's 2-digit hex directory (eg.
/objects/a1/) or the complete object location (eg. /objects/a1/b2).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 http-push.c |   58 +++++++++++++++++++---------------------------------------
 1 files changed, 19 insertions(+), 39 deletions(-)

diff --git a/http-push.c b/http-push.c
index cb5bf95..715954e 100644
--- a/http-push.c
+++ b/http-push.c
@@ -177,6 +177,16 @@ struct remote_ls_ctx
 	struct remote_ls_ctx *parent;
 };

+static char *get_remote_object_url(const char *url, const char *hex,
int only_two_digit_postfix) {
+	struct strbuf buf = STRBUF_INIT;
+
+	strbuf_addf(&buf, "%sobjects/%.*s/", url, 2, hex);
+	if(!only_two_digit_postfix)
+		strbuf_addf(&buf, "%s", hex+2);
+
+	return strbuf_detach(&buf, NULL);
+}
+
 static void finish_request(struct transfer_request *request);
 static void release_request(struct transfer_request *request);

@@ -222,8 +232,6 @@ static void start_fetch_loose(struct
transfer_request *request)
 	char *hex = sha1_to_hex(request->obj->sha1);
 	char *filename;
 	char prevfile[PATH_MAX];
-	char *url;
-	char *posn;
 	int prevlocal;
 	unsigned char prev_buf[PREV_BUF_SIZE];
 	ssize_t prev_read = 0;
@@ -273,17 +281,7 @@ static void start_fetch_loose(struct
transfer_request *request)

 	git_SHA1_Init(&request->c);

-	url = xmalloc(strlen(remote->url) + 50);
-	request->url = xmalloc(strlen(remote->url) + 50);
-	strcpy(url, remote->url);
-	posn = url + strlen(remote->url);
-	strcpy(posn, "objects/");
-	posn += 8;
-	memcpy(posn, hex, 2);
-	posn += 2;
-	*(posn++) = '/';
-	strcpy(posn, hex + 2);
-	strcpy(request->url, url);
+	request->url = get_remote_object_url(remote->url, hex, 0);

 	/* If a previous temp file is present, process what was already
 	   fetched. */
@@ -327,7 +325,7 @@ static void start_fetch_loose(struct
transfer_request *request)
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, request);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file);
 	curl_easy_setopt(slot->curl, CURLOPT_ERRORBUFFER, request->errorstr);
-	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+	curl_easy_setopt(slot->curl, CURLOPT_URL, request->url);
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header);

 	/* If we have successfully processed data from a previous fetch
@@ -356,16 +354,8 @@ static void start_mkcol(struct transfer_request *request)
 {
 	char *hex = sha1_to_hex(request->obj->sha1);
 	struct active_request_slot *slot;
-	char *posn;

-	request->url = xmalloc(strlen(remote->url) + 13);
-	strcpy(request->url, remote->url);
-	posn = request->url + strlen(remote->url);
-	strcpy(posn, "objects/");
-	posn += 8;
-	memcpy(posn, hex, 2);
-	posn += 2;
-	strcpy(posn, "/");
+	request->url = get_remote_object_url(remote->url, hex, 1);

 	slot = get_active_slot();
 	slot->callback_func = process_response;
@@ -480,7 +470,7 @@ static void start_put(struct transfer_request *request)
 {
 	char *hex = sha1_to_hex(request->obj->sha1);
 	struct active_request_slot *slot;
-	char *posn;
+	struct strbuf url_buf = STRBUF_INIT;
 	enum object_type type;
 	char hdr[50];
 	void *unpacked;
@@ -519,21 +509,11 @@ static void start_put(struct transfer_request *request)

 	request->buffer.buf.len = stream.total_out;

-	request->url = xmalloc(strlen(remote->url) +
-			       strlen(request->lock->token) + 51);
-	strcpy(request->url, remote->url);
-	posn = request->url + strlen(remote->url);
-	strcpy(posn, "objects/");
-	posn += 8;
-	memcpy(posn, hex, 2);
-	posn += 2;
-	*(posn++) = '/';
-	strcpy(posn, hex + 2);
-	request->dest = xmalloc(strlen(request->url) + 14);
-	sprintf(request->dest, "Destination: %s", request->url);
-	posn += 38;
-	*(posn++) = '_';
-	strcpy(posn, request->lock->token);
+	strbuf_addf(&url_buf, "Destination: %s",
get_remote_object_url(remote->url, hex, 0));
+	request->dest = strbuf_detach(&url_buf, NULL);
+
+	strbuf_addf(&url_buf, "%s_%s", get_remote_object_url(remote->url,
hex, 0), request->lock->token);
+	request->url = strbuf_detach(&url_buf, NULL);

 	slot = get_active_slot();
 	slot->callback_func = process_response;
-- 
1.6.0.4

^ permalink raw reply related

* Re: [PATCH] Use time_t for timestamps returned by approxidate()  instead of unsigned
From: Tim Henigan @ 2009-01-24 16:13 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0901240726070.13232@racer>

On Sat, Jan 24, 2009 at 1:27 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi,
>
> On Thu, 22 Jan 2009, Tim Henigan wrote:
>
> > Use time_t for timestamps returned by approxidate() instead of unsigned
> > long.  All references to approxidate were checked as well as references
> > to OPT_DATE.
>
> Hmm.  I vaguely remember Linus mentioning recently that unsigned long is
> the appropriate data type for the Unix Epoch...

You are correct. I just found a post on this list where Linus
specifically rejected a similar patch from another contributor.  A
quote from https://kerneltrap.org/mailarchive/git/2008/11/6/4014124:

    " "time_t" is one of those totally broken unix types. The
standards say that
    it's an "arithmetic" type, but leaves it open to be just about anything.
    Traditionally, it's a signed integer (bad), and in theory it could even be
    a floating point value, I think.

    And in _all_ such cases, it's actually better to cast it to "unsigned
    long" than keep time in a system-dependent format that is most likely
    either _already_ "unsigned long", or alternatively broken."

Should I update the GitWiki page to remove this Janitor task or do you
keep it as a test to see if people are properly searching the mail
archives?

Sorry to waste your time on the patch review.

Thanks,
Tim

^ permalink raw reply

* Re: [PATCH 0/4] gitweb feed metadata tuneups
From: Jakub Narebski @ 2009-01-24 16:02 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git
In-Reply-To: <1232686121-1800-1-git-send-email-giuseppe.bilotta@gmail.com>

On Fri, 23 Jan 2009, Giuseppe Bilotta wrote:

> The next four patches add some metadata to gitweb generated feeds:
> channel image, managing editor and last-update dates are added to RSS
> feeds, and the feed generator (gitweb, with version specification) is
> added to both RSS and Atom feeds.
> 
> Giuseppe Bilotta (4):
>   gitweb: channel image in rss feed
>   gitweb: feed generator metadata
>   gitweb: rss feed managingEditor
>   gitweb: rss channel date
> 
>  gitweb/gitweb.perl |   20 +++++++++++++++++++-
>  1 files changed, 19 insertions(+), 1 deletions(-)

I like this series; however I do not use gitweb feeds (Atom or RSS),
so I cannot say anything on their validity and usefullness.

P.S. I tried to look up who is responsible (who have added) RSS code,
but unfortunately it looks like it dates back to Kay Sievers.
Unfortunately because IIRC he is not active on the list...
-- 
Jakub Narebski
Poland

^ permalink raw reply

* [PATCH] Makefile: Use libc strlcpy on OSX
From: Benjamin Kramer @ 2009-01-24 15:41 UTC (permalink / raw)
  To: git; +Cc: gitster

OSX supports strlcpy(3) since 10.2 so we don't need
to use our own.

Signed-off-by: Benjamin Kramer <benny.kra@gmail.com>
---
 Makefile |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index b4d9cb4..4c9aab5 100644
--- a/Makefile
+++ b/Makefile
@@ -643,7 +643,6 @@ ifeq ($(uname_S),Darwin)
 	ifneq ($(shell expr "$(uname_R)" : '9\.'),2)
 		OLD_ICONV = UnfortunatelyYes
 	endif
-	NO_STRLCPY = YesPlease
 	NO_MEMMEM = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
 endif
-- 
1.6.1.285.g3454

^ permalink raw reply related

* Re: [PATCH 1/2] handle color.ui at a central place
From: Markus Heidelberg @ 2009-01-24 14:23 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Jeff King, René Scharfe, git
In-Reply-To: <alpine.DEB.1.00.0901241513460.13232@racer>

Johannes Schindelin, 24.01.2009:
> Hi,
> 
> On Sat, 24 Jan 2009, Markus Heidelberg wrote:
> 
> > format-patch is perhaps the only place where the commit has broken 
> > things, because I didn't find other places, where color config options 
> > were set
(and evaluated)
> > , but not the corresponding variables. So it seems as if only 
> > format-patch needed code like this to turn off the colors.
> 
> So you want to add a test case when you resubmit your patch...

I'm not sure, whether it should be resubmitted at all. As Jeff pointed
out, there should be a better way to clean up the color (and color.ui)
handling.

Markus

^ permalink raw reply

* Re: [PATCH v2] Change octal literals to be XEmacs friendly
From: malc @ 2009-01-24 14:18 UTC (permalink / raw)
  To: git
In-Reply-To: <7v3af9p9lq.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> malc@pulsesoft.com writes:
>
>> Kalle Olavi Niemitalo <kon@iki.fi> writes:
>>
>>> Vassili Karpov <av1474@comtv.ru> writes:
>>>
>>>> #ooctal syntax on the other hand produces integers everywhere.
>>>
>>> GNU Emacs 20.7 doesn't support #o, but neither does it include
>>
>> Bummer
>
> To be portable you could certainly spell them in decimal integers, no?

Sure.

-- 
mailto:av1474@comtv.ru

^ permalink raw reply

* Re: [PATCH 1/2] handle color.ui at a central place
From: Johannes Schindelin @ 2009-01-24 14:14 UTC (permalink / raw)
  To: Markus Heidelberg; +Cc: Junio C Hamano, Jeff King, René Scharfe, git
In-Reply-To: <200901241228.33690.markus.heidelberg@web.de>

Hi,

On Sat, 24 Jan 2009, Markus Heidelberg wrote:

> format-patch is perhaps the only place where the commit has broken 
> things, because I didn't find other places, where color config options 
> were set, but not the corresponding variables. So it seems as if only 
> format-patch needed code like this to turn off the colors.

So you want to add a test case when you resubmit your patch...

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 1/2] user-manual: Simplify the user configuration.
From: Felipe Contreras @ 2009-01-24 14:04 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Hannu Koivisto, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901241438370.13232@racer>

On Sat, Jan 24, 2009 at 3:42 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Sat, 24 Jan 2009, Felipe Contreras wrote:
>
>> On Thu, Jan 22, 2009 at 8:59 PM, Hannu Koivisto <azure@iki.fi> wrote:
>> > Felipe Contreras <felipe.contreras@gmail.com> writes:
>> >
>> >> On Thu, Jan 22, 2009 at 6:17 PM, Hannu Koivisto <azure@iki.fi> wrote:
>> >>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>> >>>
>> >>>> This brings back my previous question: where is the home directory in
>> >>>> a Windows system?
>> >>>
>> >>> It's where %HOMEDRIVE%%HOMEPATH% points to.
>> >>
>> >> I thought it was something like that. Do we want something like that
>> >> in the manual, or should we assume Windows users know that?
>> >
>> > I should have added that Unix programs (i.e. Cygwin programs and
>> > even some native ports) probably use %HOME% which may be different
>> > from %HOMEDRIVE%%HOMEPATH%.  I recall that if you haven't
>> > explicitly set up HOME in Windows environment, Cygwin sets it up
>> > magically from passwd or falls back to %HOMEDRIVE%%HOMEPATH%.  I
>> > have no idea if msysgit respects %HOME% if it is set or always uses
>> > %HOMEDRIVE%%HOMEPATH% or something completely different (user
>> > profile, most likely).
>> >
>> > It certainly may be that "home directory" is a foreign concept to
>> > some Windows users.  Some might know it as a user profile or a
>> > personal folder (just guessing, I'm pretty isolated from less
>> > experienced Windows users), even though user profile is a separate
>> > concept from "home directory" (note that there is %USERPROFILE%
>> > which by default is the same as %HOMEDRIVE%%HOMEPATH% at least in
>> > XP).
>> >
>> > In any case, what Cygwin git does should be expected by Cygwin
>> > users.  If msysgit wanted to be a really native Windows application
>> > and store the configuration where Microsoft thinks it should be
>> > stored, it probably shouldn't store the config under "home
>> > directory" to begin with (I'm guessing that's what it does) but
>> > under %USERPROFILE\Application Data\Git (...FILE\Local
>> > Settings\... in case non-roaming storage is wanted).  And in that
>> > case the manual might be misleading for msysgit users.  See
>> > e.g. <http://msdn.microsoft.com/en-us/library/ms995853.aspx>.
>>
>> Isn't that enough argument to stop assuming the user knows where is
>> the "home directory"?
>
> Why does it appear as if we always have to bend over for Windows?  That is
> really frustrating.
>
> In any case, the only thing the user would need to know the location of
> $HOME for is for .gitconfig.  And for Windows users I suggest using the
> dialog in git gui (for the variables that are editable there), which
> should cover most of what the user needs.  For everything else, they
> should use "git config".
>
> Because if you really start with explaining where the home can be on
> Windows, you would also have to cover why "%USERPROFILE%" does not work in
> Git bash.  And before you know what is happening, you have a big ass
> Windows chapter in the user manual that revolves around anything except
> Git.

Huh? That was exactly my point.

It's much easier for everyone to just use git config --global than
explain how create and edit the .gitconfig file. If this is explained
it shouldn't be in the "Telling git your name" section.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [RFC/PATCH v3 3/3] archive.c: add basic support for submodules
From: Johannes Schindelin @ 2009-01-24 13:51 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: git, Junio C Hamano, René Scharfe
In-Reply-To: <8c5c35580901240044y452b465fj94df82fc2b8f7ee9@mail.gmail.com>

Hi,

On Sat, 24 Jan 2009, Lars Hjemli wrote:

> On Fri, Jan 23, 2009 at 20:57, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> >  in all of Git, we try to assume that only reachable objects are valid 
> >  objects.
> 
> I don't think this is true (most git commands accepts their arguments
> as valid objects without verifying if they are reachable from a ref).

The fact that a user can ask for some object directly, and that we do not 
try to validate it in that case has nothing to do with said assumption.

If something is pushed to a remote, and the connection fails, some commit 
could be pushed already, but some of its reachable objects lacking.

The user on the remote side can still try to salvage parts by accessing 
the objects directly, by their name.

But the only guarantee that the objects are reachable is to start from a 
ref.

Concretely, if your patch is applied as-is, such a half-pushed state could 
affect git-archive in a nasty way: even if the user started from a ref, 
there could be missing objects!

> Do you feel it is necessary to perform a reachability check of the 
> gitlink'd commit before traversing into a submodule tree?

No.  Because HEAD is a ref, too.

Now, there is still a problem when your submodule is missing the objects 
for the commit your superproject is referring to.

IMO that is a serious issue, as it just asks for confused users.

> > - presence of a specific commit in the supermodule is a _lousy_ 
> >   indicator that the user wants to include that submodule in the 
> >   archive.
> 
> This is the issue I tried to address with my
> `--submodules=[a|c|r][g:<name>]` proposal in the commit message for
> this patch.

Nope, doing this "in the future" does not please me one bit.

Besides, I find the semantics, uhm, "interesting".  (The other word would 
be "unintuitive".  Why do you have to be so cryptic that I have to read 
the proposal to understand what the heck "c" is about?)

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 1/2] user-manual: Simplify the user configuration.
From: Johannes Schindelin @ 2009-01-24 13:42 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Hannu Koivisto, Junio C Hamano, git
In-Reply-To: <94a0d4530901240021u65adeff8pb6995ef707bc1f68@mail.gmail.com>

Hi,

On Sat, 24 Jan 2009, Felipe Contreras wrote:

> On Thu, Jan 22, 2009 at 8:59 PM, Hannu Koivisto <azure@iki.fi> wrote:
> > Felipe Contreras <felipe.contreras@gmail.com> writes:
> >
> >> On Thu, Jan 22, 2009 at 6:17 PM, Hannu Koivisto <azure@iki.fi> wrote:
> >>> Felipe Contreras <felipe.contreras@gmail.com> writes:
> >>>
> >>>> This brings back my previous question: where is the home directory in
> >>>> a Windows system?
> >>>
> >>> It's where %HOMEDRIVE%%HOMEPATH% points to.
> >>
> >> I thought it was something like that. Do we want something like that
> >> in the manual, or should we assume Windows users know that?
> >
> > I should have added that Unix programs (i.e. Cygwin programs and
> > even some native ports) probably use %HOME% which may be different
> > from %HOMEDRIVE%%HOMEPATH%.  I recall that if you haven't
> > explicitly set up HOME in Windows environment, Cygwin sets it up
> > magically from passwd or falls back to %HOMEDRIVE%%HOMEPATH%.  I
> > have no idea if msysgit respects %HOME% if it is set or always uses
> > %HOMEDRIVE%%HOMEPATH% or something completely different (user
> > profile, most likely).
> >
> > It certainly may be that "home directory" is a foreign concept to
> > some Windows users.  Some might know it as a user profile or a
> > personal folder (just guessing, I'm pretty isolated from less
> > experienced Windows users), even though user profile is a separate
> > concept from "home directory" (note that there is %USERPROFILE%
> > which by default is the same as %HOMEDRIVE%%HOMEPATH% at least in
> > XP).
> >
> > In any case, what Cygwin git does should be expected by Cygwin
> > users.  If msysgit wanted to be a really native Windows application
> > and store the configuration where Microsoft thinks it should be
> > stored, it probably shouldn't store the config under "home
> > directory" to begin with (I'm guessing that's what it does) but
> > under %USERPROFILE\Application Data\Git (...FILE\Local
> > Settings\... in case non-roaming storage is wanted).  And in that
> > case the manual might be misleading for msysgit users.  See
> > e.g. <http://msdn.microsoft.com/en-us/library/ms995853.aspx>.
> 
> Isn't that enough argument to stop assuming the user knows where is
> the "home directory"?

Why does it appear as if we always have to bend over for Windows?  That is 
really frustrating.

In any case, the only thing the user would need to know the location of 
$HOME for is for .gitconfig.  And for Windows users I suggest using the 
dialog in git gui (for the variables that are editable there), which 
should cover most of what the user needs.  For everything else, they 
should use "git config".

Because if you really start with explaining where the home can be on 
Windows, you would also have to cover why "%USERPROFILE%" does not work in 
Git bash.  And before you know what is happening, you have a big ass 
Windows chapter in the user manual that revolves around anything except 
Git.

Ciao,
Dscho

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox