Git development
 help / color / mirror / Atom feed
* Re: [PATCH 0/4] diff text conversion filter
From: Jeff King @ 2008-10-13  1:29 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Matthieu Moy, git
In-Reply-To: <48EB8B1C.9060201@viscovery.net>

On Tue, Oct 07, 2008 at 06:15:24PM +0200, Johannes Sixt wrote:

> and that should be sufficient. I'm proposing this heuristics:
> 
>  * If only textconv is given, all porcelains pick it.
>  * If only command is given, all porcelains pick it.
>  * If both are given, then
>    - git log picks textconv.
>    - git show and git diff:
>      . if exactly one pathspec was given, pick command;
>      . otherwise pick textconv

I am not 100% convinced that is sufficient, given Matthieu's other post.
But I will not personally be using external diff at all, so I am hard
pressed to come up with a counter example.

> Plumbing never picks any of them, just like today, nor should git
> format-patch. The are other porcelains that could be sorted into this
> list, like git blame and (the summary line of) git commit.

Actually, I think blaming a text-conv'd version would be useful (at
least for my case). But cleary it should be optional, and probably
defaulting to off.

Once again, I think this is a good reason to move to an explicit diff
option for "respect textconv" rather than relying on maybe or maybe not
loading the config.

> BTW, please don't take git-gui as an example that would lauch MS-Word on
> each diff. (Neither would gitk.) Both rely on plumbing, and that's good.
> gitk has a menu entry "External diff", where the diff.doc.command could be
> hooked into.

But they are both porcelain. So in theory, they could behave differently
depending on what config is available (or if not them, some other
third-party porcelain). But I was just responding to Matthieu's
use-case, so I am not sure what exactly people prefer; I am lucky enough
not to version any MS-Word documents at all. :)

-Peff

^ permalink raw reply

* Re: Fwd: git status options feature suggestion
From: Shawn O. Pearce @ 2008-10-13  1:30 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, Michael J Gruber, Johannes Schindelin,
	Caleb Cushing, git
In-Reply-To: <20081013010415.GB3768@coredump.intra.peff.net>

Jeff King <peff@peff.net> wrote:
> However, now that Shawn has revealed the existence of his super-secret
> status replacement, I am going to wait to see that before moving any
> further. :)

I'll talk about it more at the GitTogether.  I expect the sources
to be published and available for download between now and then.
I'll post a pointer to it here on the Git ML once the sources
go live.

So you shouldn't have to wait too much longer.

But as I said, it really is just a trivial redisplay of what
diff-files and diff-index produces.  Anyone could probably code up
the same result in 15 minutes in Perl or Python; or slightly longer
in C.

-- 
Shawn.

^ permalink raw reply

* Re: What's cooking in git/spearce.git (Oct 2008, #02; Sun, 12)
From: Junio C Hamano @ 2008-10-13  2:05 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081012212543.GG4856@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> * pb/rename-rowin32 (Fri Oct 3 12:20:43 2008 +0200) 1 commit
>  - Do not rename read-only files during a push
>
> Supposedly fixes pack file renames on Windows, but it makes the
> test suite fail on Linux.  I haven't debugged why yet.

I am kind of surprised that it actually passes the test on Windows, which
implies that these cats shown in the patch to fix this breakage below do
not honor ro-ness of the file, which in turn makes me doubt if making the
resulting packfiles read-only has any effect on that platform whatsoever..

 t/t5300-pack-object.sh |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git i/t/t5300-pack-object.sh w/t/t5300-pack-object.sh
index b335c6b..544f59e 100755
--- i/t/t5300-pack-object.sh
+++ w/t/t5300-pack-object.sh
@@ -248,6 +248,7 @@ test_expect_success \
      git index-pack test-3.pack &&
      cmp test-3.idx test-1-${packname_1}.idx &&
 
+     rm -f test-3.pack &&
      cat test-2-${packname_2}.pack >test-3.pack &&
      git index-pack -o tmp.idx test-2-${packname_2}.pack &&
      cmp tmp.idx test-2-${packname_2}.idx &&
@@ -255,6 +256,7 @@ test_expect_success \
      git index-pack test-3.pack &&
      cmp test-3.idx test-2-${packname_2}.idx &&
 
+     rm -f test-3.pack &&
      cat test-3-${packname_3}.pack >test-3.pack &&
      git index-pack -o tmp.idx test-3-${packname_3}.pack &&
      cmp tmp.idx test-3-${packname_3}.idx &&

^ permalink raw reply related

* Re: [JGIT PATCH 4/4] Intelligent parsing of ambiguously encoded meta data.
From: Shawn O. Pearce @ 2008-10-13  2:27 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1223851860-13068-5-git-send-email-robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> We cannot trust meta data to be encoded in any particular way, so we try
> different encodings. First we try UTF-8, which is the only sane encoding
> for non-local data, even when used in regions where eight bit legacy
> encodings are common. The chance of mistakenly parsing non-UTF-8 data
> as valid UTF-8 is varies from extremely low (western encodings) to low
> for most other encodings. If the data does not look like UTF-8, we try the
> suggested encoding. If that fails we try the user locale and finally, if
> that fails we try ISO-8859-1, which cannot fail.

Hmm.  I'm concerned about the infinite loop you have here.
If ISO-8859-1 fails we'd be stuck here until the end of time.
Plus its a bit ugly to read.

I wonder if this is any better.  It passes your tests and is 2
lines shorter.

diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
index a31734b..6c0e339 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
@@ -42,7 +42,10 @@
 import static org.spearce.jgit.lib.ObjectChecker.encoding;
 
 import java.nio.ByteBuffer;
+import java.nio.charset.CharacterCodingException;
 import java.nio.charset.Charset;
+import java.nio.charset.CharsetDecoder;
+import java.nio.charset.CodingErrorAction;
 import java.util.Arrays;
 
 import org.spearce.jgit.lib.Constants;
@@ -376,7 +379,10 @@ public static PersonIdent parsePersonIdent(final byte[] raw, final int nameB) {
 	}
 
 	/**
-	 * Decode a region of the buffer under the specified character set.
+	 * Decode a region of the buffer under the specified character set if possible.
+	 *
+	 * If the byte stream cannot be decoded that way, the platform default is tried
+	 * and if that too fails, the fail-safe ISO-8859-1 encoding is tried.
 	 * 
 	 * @param cs
 	 *            character set to use when decoding the buffer.
@@ -393,7 +399,56 @@ public static PersonIdent parsePersonIdent(final byte[] raw, final int nameB) {
 	public static String decode(final Charset cs, final byte[] buffer,
 			final int start, final int end) {
 		final ByteBuffer b = ByteBuffer.wrap(buffer, start, end - start);
-		return cs.decode(b).toString();
+		b.mark();
+
+		// Try our built-in favorite. The assumption here is that
+		// decoding will fail if the data is not actually encoded
+		// using that encoder.
+		//
+		try {
+			return decode(b, Constants.CHARSET);
+		} catch (CharacterCodingException e) {
+			b.reset();
+		}
+
+		if (!cs.equals(Constants.CHARSET)) {
+			// Try the suggested encoding, it might be right since it was
+			// provided by the caller.
+			//
+			try {
+				return decode(b, cs);
+			} catch (CharacterCodingException e) {
+				b.reset();
+			}
+		}
+
+		// Try the default character set. A small group of people
+		// might actually use the same (or very similar) locale.
+		//
+		final Charset defcs = Charset.defaultCharset();
+		if (!defcs.equals(cs) && !defcs.equals(Constants.CHARSET)) {
+			try {
+				return decode(b, defcs);
+			} catch (CharacterCodingException e) {
+				b.reset();
+			}
+		}
+
+		// Fall back to an ISO-8859-1 style encoding. At least all of
+		// the bytes will be present in the output.
+		//
+		final StringBuilder r = new StringBuilder(end - start);
+		for (int i = start; i < end; i++)
+			r.append((char) (buffer[i] & 0xff));
+		return r.toString();
+	}
+
+	private static String decode(final ByteBuffer b, final Charset charset)
+			throws CharacterCodingException {
+		final CharsetDecoder d = charset.newDecoder();
+		d.onMalformedInput(CodingErrorAction.REPORT);
+		d.onUnmappableCharacter(CodingErrorAction.REPORT);
+		return d.decode(b).toString();
 	}
 
 	/**


-- 
Shawn.

^ permalink raw reply related

* Re: [JGIT PATCH 3/4] The git config file is case insensitive
From: Shawn O. Pearce @ 2008-10-13  2:36 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1223851860-13068-4-git-send-email-robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
> index 45c2f8a..7a34cde 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
> @@ -682,7 +683,12 @@ public void load() throws IOException {
>  
>  	private void clear() {
>  		entries = new ArrayList<Entry>();
> -		byName = new HashMap<String, Object>();
> +		byName = new TreeMap<String, Object>(new Comparator<String>() {
> +
> +			public int compare(String o1, String o2) {
> +				return o1.compareToIgnoreCase(o2);
> +			}
> +		});
>  	}

This isn't necessary.  Everyone who does a get or a put against the
byName map already is forming a lower case key string.  I'd rather
keep the lookup O(1) than O(log N), especially if the code has a
ton of .toLowerCase() calls in it to normalize the keys.

If you are going to change it to a TreeMap with a custom Comparator
then maybe we should cleanup the code that operates on byName so it
can use the original input strings, instead of the .toLowerCase()
forms.

For now I'm going to apply your patch without this one hunk.  If you
want to switch to a TreeMap lets also cleanup the get/put calls.
  
-- 
Shawn.

^ permalink raw reply

* Re: What's cooking in git/spearce.git (Oct 2008, #02; Sun, 12)
From: Junio C Hamano @ 2008-10-13  3:49 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <7v3aj1fdw2.fsf@gitster.siamese.dyndns.org>

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

> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
>> * pb/rename-rowin32 (Fri Oct 3 12:20:43 2008 +0200) 1 commit
>>  - Do not rename read-only files during a push
>>
>> Supposedly fixes pack file renames on Windows, but it makes the
>> test suite fail on Linux.  I haven't debugged why yet.
>
> I am kind of surprised that it actually passes the test on Windows, which
> implies that these cats shown in the patch to fix this breakage below do
> not honor ro-ness of the file, which in turn makes me doubt if making the
> resulting packfiles read-only has any effect on that platform whatsoever..

Actually, the patch I posted was wrong and shouldn't be applied, as the
original breakage is a sign that the externally observable behaviour of
the command has changed, and the patch was just hiding it under the rug.

The thing is that 8c76006 (Do not rename read-only files during a push,
2008-10-03) makes unnecessary chmod() when there is no rename is involved,
namely when it is used to index an existing packfile on the filesystem.

I think the attached patch is a better fix, to be squashed in.

Note: I now have the pumpkin, so you do not have to apply and push the
results out --- this is just me showing a proposed change as usual.

 index-pack.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git c/index-pack.c w/index-pack.c
index 2c69f08..aec11cb 100644
--- c/index-pack.c
+++ w/index-pack.c
@@ -823,7 +823,8 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
 		if (move_temp_to_file(curr_pack_name, final_pack_name))
 			die("cannot store pack file");
 	}
-	chmod(final_pack_name, 0444);
+	if (from_stdin)
+		chmod(final_pack_name, 0444);
 
 	if (final_index_name != curr_index_name) {
 		if (!final_index_name) {

^ permalink raw reply related

* Re: [PATCH 3/4] diff: introduce diff.<driver>.binary
From: Junio C Hamano @ 2008-10-13  4:00 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Sixt, Matthieu Moy, git
In-Reply-To: <20081013012311.GE3768@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

>> The logic behind the original behaviour was that the file ought to be
>> "diff-able" if you are setting up funcname pattern because the funcname
>> pattern only makes sense if you are doing the textual diff.  In other
>> words, "should we do textual diff?" and "what funcname pattern should we
>> use?" are _not_ orthogonal, as wanting to configure the latter does imply
>> that you do want to see the change in the textual diff format.
>
> Yeah, I don't think I can really disagree with that. I had some vague
> notion that it opens the path for adding orthogonal options later. But
> really, I'm not sure that any exist, since they are, by definition
> related to the diff. Unless we want to have diff driver options for how
> to do a binary diff.

I actually have an independent worry about this whole thing.

The textconv filter is primarily for humans to be able to view the diff,
as far as I understand it, but what would this facility do to the patch
exchange workflow?  There needs either one or both of the following:

 - A command line option to Porcelains to override textconv so that an
   applicable binary diff can be obtained upon request (or format-patch
   always disables textconv); and/or

 - You teach git-apply to use a reverse transformation of textconv, so
   that it does, upon reception of a textconv diff:

   (1) pass existing preimage through textconv;
   (2) apply the patch;
   (3) convert the result back to binary.

Even if we do not implement the latter right now in the first round, if we
want to leave the door open for later for it, we would somehow need to
mark such a patch between textconv output so that what kind of reverse
transformation needs to be applied.

I'd say that format-patch should always disable textconv so that we won't
have to worry about it for now.  If we enable textconv for format-patch,
we do need to include what textconv filter (or "diff attribute") was used
for the path in the output, so that the receiving end can pick a
corresponding reverse transformation.

^ permalink raw reply

* Re: tip tree clone fail
From: H. Peter Anvin @ 2008-10-13  4:07 UTC (permalink / raw)
  To: Petr Baudis
  Cc: Ingo Molnar, Wang Chen, Thomas Gleixner, FNST-Lai Jiangshan,
	FJ-KOSAKI Motohiro, git, Junio C Hamano
In-Reply-To: <20081012153952.GV10544@machine.or.cz>

Petr Baudis wrote:
> On Sun, Oct 12, 2008 at 05:24:27PM +0200, Ingo Molnar wrote:
>> hm, -tip's .git/hooks/post-update already contained this, for the last 2 
>> months:
>>
>>   exec git update-server-info
>>
>> so ... _despite_ us having this in the git repo, the HTTP protocol still 
>> does not work. Why?
> 
> I think your problem is that HTTP does not know where to look for
> objects coming from alternates; IIRC this would work if you used
> relative paths in objects/info/alternates, or you can create
> objects/info/http-alternates like
> 
> 	/pub/scm/linux/kernel/git/torvalds/linux-2.6.git/objects
> 	/pub/scm/linux/kernel/git/sfr/linux-next.git/objects
> 

Yes, alternates must not contain paths referencing /home/ftp.

	-hpa

^ permalink raw reply

* Re: tip tree clone fail
From: H. Peter Anvin @ 2008-10-13  4:09 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Petr Baudis, Wang Chen, Thomas Gleixner, FNST-Lai Jiangshan,
	FJ-KOSAKI Motohiro, git, Junio C Hamano
In-Reply-To: <20081012165954.GA2317@elte.hu>

Ingo Molnar wrote:
> 
> Soapbox: in fact it would be outright stupid to limit the kernel 
> source's availability artificially by not making HTTP a tier-one access 
> method.
> 
> Fighting against HTTP-only firewalls is like constantly pointing it out 
> to the popular press that they should say 'cracker' instead of 'hacker'. 
> It is pointless and only hurts the availability our own project.
> 	

Yes, and Shawn Pierce is working on making HTTP a proper "smart" access 
method, based on a proposal I submitted.  That is the proper solution, 
since it avoids all the braindamage of "dumb" access methods.  We 
[kernel.org] intend to deploy it as soon as it is available.

	-hpa

^ permalink raw reply

* Re: [PATCH 3/4] diff: introduce diff.<driver>.binary
From: Jeff King @ 2008-10-13  4:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, Matthieu Moy, git
In-Reply-To: <7vk5cddtzh.fsf@gitster.siamese.dyndns.org>

On Sun, Oct 12, 2008 at 09:00:50PM -0700, Junio C Hamano wrote:

> exchange workflow?  There needs either one or both of the following:
> 
>  - A command line option to Porcelains to override textconv so that an
>    applicable binary diff can be obtained upon request (or format-patch
>    always disables textconv); and/or

format-patch should always disable textconv. I admit that I didn't test
it, but assumed it was covered under the same code path as external
diff (i.e., just not reading in the config). But it doesn't seem to be.

So that is definitely broken in my patch series. But even once that is
fixed, I agree there should be a porcelain switch for turning this off.
I sometimes do "git diff >patch" and read the result into my MUA.
Obviously I would not want textconv'ing there.

>  - You teach git-apply to use a reverse transformation of textconv, so
>    that it does, upon reception of a textconv diff:
> 
>    (1) pass existing preimage through textconv;
>    (2) apply the patch;
>    (3) convert the result back to binary.

The problem with this approach is that it requires that the textconv be
a reversible mapping. And the two motivating examples (dumping exif tags
and converting word processor documents to text) are not; they are lossy
conversions.

It's possible that one could, given the binary preimage and the two
lossy textconv'd versions, produce a custom binary merge that would just
munge the tags, or just munge the text, or whatever. But that is an
order of magnitude more work than writing a textconv, which is usually
as simple as "/path/to/existing/conversion-script".

And the whole point of this exercise was to make it simple to set this
conversion up.

> I'd say that format-patch should always disable textconv so that we won't
> have to worry about it for now.

Agreed, if you remove "for now". I had never intended for these to be
anything but a human-readable display (and while I am generally OK with
generalizing functionality when we can, I think there is real value in
the simplicity here).

If somebody really wants to send patches with converted text for
reference, I would suggest producing a patch with the textconv'd output
as a comment, and including the full binary patch to actually be
applied (and yes, obviously they a malicious attacker could make them
not match, but given the binary patch, we can trivially regenerate the
textconv'd version and confirm it).

-Peff

^ permalink raw reply

* Re: [PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true
From: Mark Levedahl @ 2008-10-13  4:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: spearce, dpotapov, git
In-Reply-To: <7vd4i5fkny.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> I was wondering if git_cygwin_config() was originally written not to call
> git_default_config() because some command implementations do not to want
> to call git_default_core_config() (and/or read trust_executable_bit
> variable from the configuration) for some reason (which would be just
> hiding bugs in other parts of the system, I suspect).
>
> If that is the case, we would have to fix such broken parts of the system,
> but until that happens your original patch to use a separate variable and
> keeping trust_executable_bit untouched would be much safer than this
> latest patch.  Hence the question.
>   
I was worried about altering the startup code, which is why I tried to 
introduce as little change as possible. However, having rewritten the 
patch using git_default_config() everything seems fine (the testsuite 
passes, or at least as well as it usually does under Cygwin, and I 
assume that is sufficient as this is Cygiwn specific). So, that patch 
follows.

Mark

^ permalink raw reply

* [PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true
From: Mark Levedahl @ 2008-10-13  4:33 UTC (permalink / raw)
  To: gitster; +Cc: spearce, dpotapov, git, Mark Levedahl
In-Reply-To: <7vd4i5fkny.fsf@gitster.siamese.dyndns.org>

Cygwin's POSIX emulation allows use of core.filemode true, unlike native
Window's implementation of stat / lstat, and Cygwin/git users who have
configured core.filemode true in various repositories will be very
unpleasantly surprised to find that git is no longer honoring that option.
So, this patch forces use of Cygwin's stat functions if core.filemode is
set true, regardless of any other considerations.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 Documentation/config.txt |    4 +++-
 compat/cygwin.c          |   20 +++++++++++++++-----
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 7161597..a3a9495 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -124,7 +124,9 @@ core.ignoreCygwinFSTricks::
 	one hierarchy using Cygwin mount. If true, Git uses native Win32 API
 	whenever it is possible and falls back to Cygwin functions only to
 	handle symbol links. The native mode is more than twice faster than
-	normal Cygwin l/stat() functions. True by default.
+	normal Cygwin l/stat() functions. True by default, unless core.filemode
+	is true, in which case ignoreCygwinFSTricks is ignored as Cygwin's
+	POSIX emulation is required to support core.filemode.
 
 core.trustctime::
 	If false, the ctime differences between the index and the
diff --git a/compat/cygwin.c b/compat/cygwin.c
index 423ff20..780375e 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -91,22 +91,32 @@ static int cygwin_stat(const char *path, struct stat *buf)
  * functions should be used. The choice is determined by core.ignorecygwinfstricks.
  * Reading this option is not always possible immediately as git_dir may be
  * not be set yet. So until it is set, use cygwin lstat/stat functions.
+ * However, if the trust_executable_bit is set, we must use the Cygwin posix
+ * stat/lstat as the Windows stat fuctions do not determine posix filemode.
  */
 static int native_stat = 1;
+extern int trust_executable_bit;
 
 static int git_cygwin_config(const char *var, const char *value, void *cb)
 {
-	if (!strcmp(var, "core.ignorecygwinfstricks"))
-		native_stat = git_config_bool(var, value);
-	return 0;
+	if (!strcmp(var, "core.ignorecygwinfstricks")) {
+			native_stat = git_config_bool(var, value);
+			return 0;
+	}
+	return git_default_config(var, value, cb);
 }
 
 static int init_stat(void)
 {
 	if (have_git_dir()) {
 		git_config(git_cygwin_config, NULL);
-		cygwin_stat_fn = native_stat ? cygwin_stat : stat;
-		cygwin_lstat_fn = native_stat ? cygwin_lstat : lstat;
+		if (!trust_executable_bit && native_stat) {
+			cygwin_stat_fn = cygwin_stat;
+			cygwin_lstat_fn = cygwin_lstat;
+		} else {
+			cygwin_stat_fn = stat;
+			cygwin_lstat_fn = lstat;
+		}
 		return 0;
 	}
 	return 1;
-- 
1.6.0.2.535.ga11fa.dirty

^ permalink raw reply related

* Re: [PATCH v2] correct verify_path for Windows
From: Johannes Sixt @ 2008-10-13  6:00 UTC (permalink / raw)
  To: Alex Riesen
  Cc: Dmitry Potapov, Joshua Juran, Giovanni Funchal, git,
	Shawn O. Pearce
In-Reply-To: <20081012181836.GA10626@steel.home>

Alex Riesen schrieb:
> I looked at the callers (briefly). Performance could be a problem: add
> and checkout can work with real big file lists and long pathnames.
> So ok, than. It is critical.

You are kidding, aren't you? What you win by a few CPU instructions here
is dwarfed by the time that the stat() implementation requires. Dmitry,
please use the more readable tolower().

-- Hannes

^ permalink raw reply

* Re: [PATCH 3/4] diff: introduce diff.<driver>.binary
From: Johannes Sixt @ 2008-10-13  6:10 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Matthieu Moy, git
In-Reply-To: <20081013041525.GA32629@coredump.intra.peff.net>

Jeff King schrieb:
> On Sun, Oct 12, 2008 at 09:00:50PM -0700, Junio C Hamano wrote:
>>  - You teach git-apply to use a reverse transformation of textconv, so
>>    that it does, upon reception of a textconv diff:
>>
>>    (1) pass existing preimage through textconv;
>>    (2) apply the patch;
>>    (3) convert the result back to binary.
> 
> The problem with this approach is that it requires that the textconv be
> a reversible mapping. And the two motivating examples (dumping exif tags
> and converting word processor documents to text) are not; they are lossy
> conversions.
> 
> It's possible that one could, given the binary preimage and the two
> lossy textconv'd versions, produce a custom binary merge that would just
> munge the tags, or just munge the text, or whatever. But that is an
> order of magnitude more work than writing a textconv, which is usually
> as simple as "/path/to/existing/conversion-script".

I fully agree with you. .texconv should only be used for human
consumption. We already have a reversible binary<->text conversion: the
binary diffs.

-- Hannes

^ permalink raw reply

* Re: [PATCH] Introduce core.keepHardLinks
From: Junio C Hamano @ 2008-10-13  6:15 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, spearce
In-Reply-To: <alpine.DEB.1.00.0810111344241.22125@pacific.mpi-cbg.de.mpi-cbg.de>

Shawn had this queued in pu, but it appears to even fail its own tests on
my machine, when applied on top of 'master'.

I tentatively dropped this from pu.

^ permalink raw reply

* Re: [PATCH v2] correct verify_path for Windows
From: Alex Riesen @ 2008-10-13  6:18 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Dmitry Potapov, Joshua Juran, Giovanni Funchal, git,
	Shawn O. Pearce
In-Reply-To: <48F2E410.2080504@viscovery.net>

2008/10/13 Johannes Sixt <j.sixt@viscovery.net>:
> Alex Riesen schrieb:
>> I looked at the callers (briefly). Performance could be a problem: add
>> and checkout can work with real big file lists and long pathnames.
>> So ok, than. It is critical.
>
> You are kidding, aren't you? What you win by a few CPU instructions here
> is dwarfed by the time that the stat() implementation requires. Dmitry,
> please use the more readable tolower().

It is called for every element in a path, not just for every filename.
And maybe it is dwarfed, but they both are part of the same operation.
And this code makes it more CPU intensive.

^ permalink raw reply

* Re: What's cooking in git/spearce.git (Oct 2008, #02; Sun, 12)
From: Stephen Haberman @ 2008-10-13  6:37 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Junio C Hamano
In-Reply-To: <20081012212543.GG4856@spearce.org>


> * sh/maint-rebase3 (Sun Oct 5 23:26:52 2008 -0500) 1 commit
>  - rebase--interactive: fix parent rewriting for dropped commits
> 
> A prior version of sh/rebase-i-p.  This should be dropped.

maint-rebase3 is actually a separate issue than sh/rebase-i-p.

maint-rebase3 deals with commits dropped because they are recognized as
cherry picks, while rebase-i-p deals with commits that can be ignored
from the todo because their parents are not being changed.

I'm surprised they both applied cleanly to pu--I was expecting a
conflict, as I noted in one of my posts. I just fetched pu and they
seem to have each broke the each other's tests as well.

To fix this, I have some local changes to pu that gets both tests
passing again. Should I submit my integration changes as a diff on top
of pu? Or should I try and integrate the two patches into one series?

Technically these two fixes are different things, and I assert you
could apply one or the other, but if you apply both, they require
integration, so I'm not sure what to do.

(...nuts, t3404#22 is failing, so my maint3-rebase/rebase-i-p
integration patch is not quite ready yet.)

(I also have a test comment typo and test_expect_failure change to make
to rebase-i-p from Junio's feedback and would like to know the
preferred way to submit those--e.g. a patch on top of your pu, a patch
on top of the existing series, or a new series all together. Given it
is not next, I'm guessing a new series all together.)

Thanks,
Stephen

^ permalink raw reply

* Re: User Authentication ?
From: Neshama Parhoti @ 2008-10-13  8:09 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <m3y70vj8ag.fsf@localhost.localdomain>

Hi,

On Sat, Oct 11, 2008 at 8:28 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> "Neshama Parhoti" <pneshama@gmail.com> writes:
>
>> I want to setup a git server on the web but I need user
>> authentication.
>> I really don't want to give ssh logins for people who I just want to
>> be able to access my repository...
>
> First, you can always set git-shell as shell for those git only
> accounts.

Thank you ! That should really be good. Any chance you are aware of
documentation
or further guidance about how to set this up ?
thanks!

^ permalink raw reply

* [PATCH (GITK) v3 1/4] gitk: Port new encoding logic from git-gui.
From: Alexander Gavrilov @ 2008-10-13  8:12 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Johannes Sixt
In-Reply-To: <1223885554-27718-1-git-send-email-angavrilov@gmail.com>

Recently handling of file encodings in GIT-GUI has
been greatly enhanced. It now follows these rules:

- File encoding defaults to the system encoding.
- It can be overridden by setting the gui.encoding option.
- Finally, the 'encoding' attribute is checked on
  per-file basis; it has the last word.

This patch inserts copies of git-gui functions necessary
for supporting this into gitk. The following patch will
connect them to the rest of the code.

Note: Since git-check-attr does not provide support for
reading attributes from trees, attribute lookup is done
using files from the working directory.

It also extends the range of supported encoding
names, to match changes in git-gui.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
Tested-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 gitk |   39 ++++++++++++++++++++++++++++++++++++---
 1 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/gitk b/gitk
index dce17e9..8682838 100755
--- a/gitk
+++ b/gitk
@@ -9727,7 +9727,7 @@ set encoding_aliases {
     { ISO-8859-16 iso-ir-226 ISO_8859-16:2001 ISO_8859-16 latin10 l10 }
     { GBK CP936 MS936 windows-936 }
     { JIS_Encoding csJISEncoding }
-    { Shift_JIS MS_Kanji csShiftJIS }
+    { Shift_JIS MS_Kanji csShiftJIS ShiftJIS Shift-JIS }
     { Extended_UNIX_Code_Packed_Format_for_Japanese csEUCPkdFmtJapanese
       EUC-JP }
     { Extended_UNIX_Code_Fixed_Width_for_Japanese csEUCFixWidJapanese }
@@ -9769,7 +9769,7 @@ proc tcl_encoding {enc} {
     set i [lsearch -exact $lcnames $enc]
     if {$i < 0} {
 	# look for "isonnn" instead of "iso-nnn" or "iso_nnn"
-	if {[regsub {^iso[-_]} $enc iso encx]} {
+	if {[regsub {^(iso|cp|ibm|jis)[-_]} $enc {\1} encx]} {
 	    set i [lsearch -exact $lcnames $encx]
 	}
     }
@@ -9781,7 +9781,7 @@ proc tcl_encoding {enc} {
 	    foreach e $ll {
 		set i [lsearch -exact $lcnames $e]
 		if {$i < 0} {
-		    if {[regsub {^iso[-_]} $e iso ex]} {
+		    if {[regsub {^(iso|cp|ibm|jis)[-_]} $e {\1} ex]} {
 			set i [lsearch -exact $lcnames $ex]
 		    }
 		}
@@ -9796,6 +9796,34 @@ proc tcl_encoding {enc} {
     return {}
 }
 
+proc gitattr {path attr default} {
+	if {[catch {set r [exec git check-attr $attr -- $path]}]} {
+		set r unspecified
+	} else {
+		set r [join [lrange [split $r :] 2 end] :]
+		regsub {^ } $r {} r
+	}
+	if {$r eq {unspecified}} {
+		return $default
+	}
+	return $r
+}
+
+proc get_path_encoding {path} {
+	global gui_encoding
+	set tcl_enc [tcl_encoding $gui_encoding]
+	if {$tcl_enc eq {}} {
+		set tcl_enc [encoding system]
+	}
+	if {$path ne {}} {
+		set enc2 [tcl_encoding [gitattr $path encoding $tcl_enc]]
+		if {$enc2 ne {}} {
+			set tcl_enc $enc2
+		}
+	}
+	return $tcl_enc
+}
+
 # First check that Tcl/Tk is recent enough
 if {[catch {package require Tk 8.4} err]} {
     show_error {} . [mc "Sorry, gitk cannot run with this version of Tcl/Tk.\n\
@@ -9818,6 +9846,11 @@ if {$tclencoding == {}} {
     puts stderr "Warning: encoding $gitencoding is not supported by Tcl/Tk"
 }
 
+set gui_encoding [encoding system]
+catch {
+	set gui_encoding [exec git config --get gui.encoding]
+}
+
 set mainfont {Helvetica 9}
 set textfont {Courier 9}
 set uifont {Helvetica 9 bold}
-- 
1.6.0.20.g6148bc

^ permalink raw reply related

* [PATCH (GITK) v3 0/4] Enhance encoding support.
From: Alexander Gavrilov @ 2008-10-13  8:12 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Johannes Sixt

Currently GUI tools don't provide enough support for
viewing files that contain non-ASCII characters. This set of
patches addresses this issue. The git-gui part of the series
is based on patches that were in the 'pu' branch of the
git-gui repository.

UPDATE: the git-gui part of the series is in master.

File encoding can be specified in the following ways:

1) It defaults to the current locale encoding.
2) It can be overridden by setting the gui.encoding option.
3) It can be further set on per-file basis by specifying
   the 'encoding' attribute in gitattributes.
4) Finally, git-gui allows directly selecting encodings
   through a popup menu.

Since git apparently cannot work with filenames in non-locale
encodings anyway, I did not try to do anything about it apart
from fixing some obvious bugs.


CHANGES v2:

- Wrote better comments for the first three patches.
- Added a patch to speed up loading of very large diffs.

CHANGES v3:

- Killed get_cached_encoding, replacing it with get_path_encoding.
- Squashed file name and content patches into one.
- Added the tcl_encoding reimplementation patch. I should
  say that without the second cache this really makes a
  difference with thousand-file diffs.

GITK:
	gitk: Port new encoding logic from git-gui.
	---
	 gitk |   39 ++++++++++++++++++++++++++++++++++++---
	 1 files changed, 36 insertions(+), 3 deletions(-)

	gitk: Enhance file encoding support.
	---
	 gitk |   35 +++++++++++++++++++++++++++--------
	 1 files changed, 27 insertions(+), 8 deletions(-)

	gitk: Implement batch lookup and caching of encoding attrs.
	---
	 gitk |   35 ++++++++++++++++++++++++++++++++++-
	 1 files changed, 34 insertions(+), 1 deletions(-)

	gitk: Optimize encoding name resolution using a lookup table.
	---
	 gitk |   84 ++++++++++++++++++++++++++++++++++++++++++-----------------------
	 1 files changed, 54 insertions(+), 30 deletions(-)

^ permalink raw reply

* [PATCH (GITK) v3 2/4] gitk: Enhance file encoding support.
From: Alexander Gavrilov @ 2008-10-13  8:12 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Johannes Sixt
In-Reply-To: <1223885554-27718-2-git-send-email-angavrilov@gmail.com>

The previous patch has added functions that implement
per-file contents encoding selection logic. This one
modifies core gitk code to use them.

Since diffs usually contain multiple files, which may
have different values of the encoding attribute, they
are initially read as binary, and encoding conversion
is applied to individual lines, based on the active
set of diff headers.

As a side job, this patch also fixes some bugs in
handling of non-ASCII filenames. Core git apparently
supports only locale-encoded filenames, so processing
is done using the system encoding.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
Tested-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 gitk |   35 +++++++++++++++++++++++++++--------
 1 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/gitk b/gitk
index 8682838..5f35f61 100755
--- a/gitk
+++ b/gitk
@@ -6229,7 +6229,7 @@ proc gettree {id} {
 	    set treepending $id
 	    set treefilelist($id) {}
 	    set treeidlist($id) {}
-	    fconfigure $gtf -blocking 0
+	    fconfigure $gtf -blocking 0 -encoding binary
 	    filerun $gtf [list gettreeline $gtf $id]
 	}
     } else {
@@ -6251,11 +6251,12 @@ proc gettreeline {gtf id} {
 	    set line [string range $line 0 [expr {$i-1}]]
 	    if {$diffids ne $nullid2 && [lindex $line 1] ne "blob"} continue
 	    set sha1 [lindex $line 2]
-	    if {[string index $fname 0] eq "\""} {
-		set fname [lindex $fname 0]
-	    }
 	    lappend treeidlist($id) $sha1
 	}
+	if {[string index $fname 0] eq "\""} {
+	    set fname [lindex $fname 0]
+	}
+	set fname [encoding convertfrom $fname]
 	lappend treefilelist($id) $fname
     }
     if {![eof $gtf]} {
@@ -6296,7 +6297,7 @@ proc showfile {f} {
 	    return
 	}
     }
-    fconfigure $bf -blocking 0
+    fconfigure $bf -blocking 0 -encoding [get_path_encoding $f]
     filerun $bf [list getblobline $bf $diffids]
     $ctext config -state normal
     clear_ctext $commentend
@@ -6334,6 +6335,7 @@ proc mergediff {id} {
     global diffids
     global parents
     global diffcontext
+    global diffencoding
     global limitdiffs vfilelimit curview
 
     set diffmergeid $id
@@ -6347,9 +6349,10 @@ proc mergediff {id} {
 	error_popup "[mc "Error getting merge diffs:"] $err"
 	return
     }
-    fconfigure $mdf -blocking 0
+    fconfigure $mdf -blocking 0 -encoding binary
     set mdifffd($id) $mdf
     set np [llength $parents($curview,$id)]
+    set diffencoding [get_path_encoding {}]
     settabs $np
     filerun $mdf [list getmergediffline $mdf $id $np]
 }
@@ -6357,6 +6360,7 @@ proc mergediff {id} {
 proc getmergediffline {mdf id np} {
     global diffmergeid ctext cflist mergemax
     global difffilestart mdifffd
+    global diffencoding
 
     $ctext conf -state normal
     set nr 0
@@ -6368,18 +6372,22 @@ proc getmergediffline {mdf id np} {
 	}
 	if {[regexp {^diff --cc (.*)} $line match fname]} {
 	    # start of a new file
+	    set fname [encoding convertfrom $fname]
 	    $ctext insert end "\n"
 	    set here [$ctext index "end - 1c"]
 	    lappend difffilestart $here
 	    add_flist [list $fname]
+	    set diffencoding [get_path_encoding $fname]
 	    set l [expr {(78 - [string length $fname]) / 2}]
 	    set pad [string range "----------------------------------------" 1 $l]
 	    $ctext insert end "$pad $fname $pad\n" filesep
 	} elseif {[regexp {^@@} $line]} {
+	    set line [encoding convertfrom $diffencoding $line]
 	    $ctext insert end "$line\n" hunksep
 	} elseif {[regexp {^[0-9a-f]{40}$} $line] || [regexp {^index} $line]} {
 	    # do nothing
 	} else {
+	    set line [encoding convertfrom $diffencoding $line]
 	    # parse the prefix - one ' ', '-' or '+' for each parent
 	    set spaces {}
 	    set minuses {}
@@ -6514,7 +6522,7 @@ proc gettreediffs {ids} {
 
     set treepending $ids
     set treediff {}
-    fconfigure $gdtf -blocking 0
+    fconfigure $gdtf -blocking 0 -encoding binary
     filerun $gdtf [list gettreediffline $gdtf $ids]
 }
 
@@ -6530,6 +6538,7 @@ proc gettreediffline {gdtf ids} {
 	    if {[string index $file 0] eq "\""} {
 		set file [lindex $file 0]
 	    }
+	    set file [encoding convertfrom $file]
 	    lappend treediff $file
 	}
     }
@@ -6587,6 +6596,7 @@ proc getblobdiffs {ids} {
     global diffcontext
     global ignorespace
     global limitdiffs vfilelimit curview
+    global diffencoding
 
     set cmd [diffcmd $ids "-p -C --no-commit-id -U$diffcontext"]
     if {$ignorespace} {
@@ -6600,7 +6610,8 @@ proc getblobdiffs {ids} {
 	return
     }
     set diffinhdr 0
-    fconfigure $bdf -blocking 0
+    set diffencoding [get_path_encoding {}]
+    fconfigure $bdf -blocking 0 -encoding binary
     set blobdifffd($ids) $bdf
     filerun $bdf [list getblobdiffline $bdf $diffids]
 }
@@ -6634,6 +6645,7 @@ proc getblobdiffline {bdf ids} {
     global diffids blobdifffd ctext curdiffstart
     global diffnexthead diffnextnote difffilestart
     global diffinhdr treediffs
+    global diffencoding
 
     set nr 0
     $ctext conf -state normal
@@ -6671,10 +6683,13 @@ proc getblobdiffline {bdf ids} {
 	    } else {
 		set fname [string range $line 2 [expr {$i - 1}]]
 	    }
+	    set fname [encoding convertfrom $fname]
+	    set diffencoding [get_path_encoding $fname]
 	    makediffhdr $fname $ids
 
 	} elseif {[regexp {^@@ -([0-9]+)(,[0-9]+)? \+([0-9]+)(,[0-9]+)? @@(.*)} \
 		       $line match f1l f1c f2l f2c rest]} {
+	    set line [encoding convertfrom $diffencoding $line]
 	    $ctext insert end "$line\n" hunksep
 	    set diffinhdr 0
 
@@ -6684,6 +6699,7 @@ proc getblobdiffline {bdf ids} {
 		if {[string index $fname 0] eq "\""} {
 		    set fname [lindex $fname 0]
 		}
+		set fname [encoding convertfrom $fname]
 		set i [lsearch -exact $treediffs($ids) $fname]
 		if {$i >= 0} {
 		    setinlist difffilestart $i $curdiffstart
@@ -6694,6 +6710,8 @@ proc getblobdiffline {bdf ids} {
 		if {[string index $fname 0] eq "\""} {
 		    set fname [lindex $fname 0]
 		}
+		set fname [encoding convertfrom $fname]
+		set diffencoding [get_path_encoding $fname]
 		makediffhdr $fname $ids
 	    } elseif {[string compare -length 3 $line "---"] == 0} {
 		# do nothing
@@ -6705,6 +6723,7 @@ proc getblobdiffline {bdf ids} {
 	    $ctext insert end "$line\n" filesep
 
 	} else {
+	    set line [encoding convertfrom $diffencoding $line]
 	    set x [string range $line 0 0]
 	    if {$x == "-" || $x == "+"} {
 		set tag [expr {$x == "+"}]
-- 
1.6.0.20.g6148bc

^ permalink raw reply related

* [PATCH (GITK) v3 4/4] gitk: Optimize encoding name resolution using a lookup table.
From: Alexander Gavrilov @ 2008-10-13  8:12 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Johannes Sixt
In-Reply-To: <1223885554-27718-4-git-send-email-angavrilov@gmail.com>

Current implementation of tcl_encoding uses linear search,
and is rather slow when called hundreds of times. This commit
reimplements it using a lookup table, which is efficiently
calculated on the first run. After that, resolution costs two
calls to [info exists], and one actual fetch from the table.

This is a port of git-gui commit a1c3feb7fac7.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 gitk |   84 ++++++++++++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 54 insertions(+), 30 deletions(-)

diff --git a/gitk b/gitk
index 8cd3171..134df00 100755
--- a/gitk
+++ b/gitk
@@ -9783,39 +9783,63 @@ set encoding_aliases {
     { Big5 csBig5 }
 }
 
-proc tcl_encoding {enc} {
-    global encoding_aliases
-    set names [encoding names]
-    set lcnames [string tolower $names]
-    set enc [string tolower $enc]
-    set i [lsearch -exact $lcnames $enc]
-    if {$i < 0} {
-	# look for "isonnn" instead of "iso-nnn" or "iso_nnn"
-	if {[regsub {^(iso|cp|ibm|jis)[-_]} $enc {\1} encx]} {
-	    set i [lsearch -exact $lcnames $encx]
+proc build_encoding_table {} {
+	global encoding_aliases encoding_lookup_table
+
+	# Prepare the lookup list; cannot use lsort -nocase because
+	# of compatibility issues with older Tcl (e.g. in msysgit)
+	set names [list]
+	foreach item [encoding names] {
+		lappend names [list [string tolower $item] $item]
+	}
+	set names [lsort -ascii -index 0 $names]
+	# neither can we use lsearch -index
+	set lnames [list]
+	foreach item $names {
+		lappend lnames [lindex $item 0]
+	}
+
+	foreach grp $encoding_aliases {
+		set target {}
+		foreach item $grp {
+			set i [lsearch -sorted -ascii $lnames \
+					[string tolower $item]]
+			if {$i >= 0} {
+				set target [lindex $names $i 1]
+				break
+			}
+		}
+		if {$target eq {}} continue
+		foreach item $grp {
+			set encoding_lookup_table([string tolower $item]) $target
+		}
 	}
-    }
-    if {$i < 0} {
-	foreach l $encoding_aliases {
-	    set ll [string tolower $l]
-	    if {[lsearch -exact $ll $enc] < 0} continue
-	    # look through the aliases for one that tcl knows about
-	    foreach e $ll {
-		set i [lsearch -exact $lcnames $e]
-		if {$i < 0} {
-		    if {[regsub {^(iso|cp|ibm|jis)[-_]} $e {\1} ex]} {
-			set i [lsearch -exact $lcnames $ex]
-		    }
+
+	foreach item $names {
+		set encoding_lookup_table([lindex $item 0]) [lindex $item 1]
+	}
+}
+
+proc tcl_encoding {enc} {
+	global encoding_lookup_table
+	if {$enc eq {}} {
+		return {}
+	}
+	if {![info exists encoding_lookup_table]} {
+		build_encoding_table
+	}
+	set enc [string tolower $enc]
+	if {![info exists encoding_lookup_table($enc)]} {
+		# look for "isonnn" instead of "iso-nnn" or "iso_nnn"
+		if {[regsub {^(iso|cp|ibm|jis)[-_]} $enc {\1} encx]} {
+			set enc $encx
 		}
-		if {$i >= 0} break
-	    }
-	    break
 	}
-    }
-    if {$i >= 0} {
-	return [lindex $names $i]
-    }
-    return {}
+	if {[info exists encoding_lookup_table($enc)]} {
+		return $encoding_lookup_table($enc)
+	} else {
+		return {}
+	}
 }
 
 proc gitattr {path attr default} {
-- 
1.6.0.20.g6148bc

^ permalink raw reply related

* [PATCH (GITK) v3 3/4] gitk: Implement batch lookup and caching of encoding attrs.
From: Alexander Gavrilov @ 2008-10-13  8:12 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Johannes Sixt
In-Reply-To: <1223885554-27718-3-git-send-email-angavrilov@gmail.com>

When the diff contains thousands of files, calling git-check-attr
once per file is very slow. With this patch gitk does attribute
lookup in batches of 30 files while reading the diff file list,
which leads to a very noticeable speedup.

It may be possible to reimplement this even more efficiently,
if git-check-attr is modified to support a --stdin-paths option.
Additionally, it should quote the ':' character in file paths,
or provide a more robust way of column separation.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
Tested-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 gitk |   35 ++++++++++++++++++++++++++++++++++-
 1 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/gitk b/gitk
index 5f35f61..8cd3171 100755
--- a/gitk
+++ b/gitk
@@ -6531,6 +6531,7 @@ proc gettreediffline {gdtf ids} {
     global cmitmode vfilelimit curview limitdiffs
 
     set nr 0
+    set sublist {}
     while {[incr nr] <= 1000 && [gets $gdtf line] >= 0} {
 	set i [string first "\t" $line]
 	if {$i >= 0} {
@@ -6540,8 +6541,10 @@ proc gettreediffline {gdtf ids} {
 	    }
 	    set file [encoding convertfrom $file]
 	    lappend treediff $file
+	    lappend sublist $file
 	}
     }
+    cache_gitattr encoding $sublist
     if {![eof $gdtf]} {
 	return [expr {$nr >= 1000? 2: 1}]
     }
@@ -9816,18 +9819,48 @@ proc tcl_encoding {enc} {
 }
 
 proc gitattr {path attr default} {
-	if {[catch {set r [exec git check-attr $attr -- $path]}]} {
+	global path_attr_cache
+	if {[info exists path_attr_cache($attr,$path)]} {
+		set r $path_attr_cache($attr,$path)
+	} elseif {[catch {set r [exec git check-attr $attr -- $path]}]} {
 		set r unspecified
 	} else {
 		set r [join [lrange [split $r :] 2 end] :]
 		regsub {^ } $r {} r
 	}
+	set path_attr_cache($attr,$path) $r
 	if {$r eq {unspecified}} {
 		return $default
 	}
 	return $r
 }
 
+proc cache_gitattr {attr pathlist} {
+	global path_attr_cache
+	set newlist {}
+	foreach path $pathlist {
+		if {[info exists path_attr_cache($attr,$path)]} continue
+		lappend newlist $path
+	}
+	while {$newlist ne {}} {
+		set head [lrange $newlist 0 29]
+		set newlist [lrange $newlist 30 end]
+		if {![catch {set rlist [eval exec git check-attr $attr -- $head]}]} {
+			foreach row [split $rlist "\n"] {
+				set cols [split $row :]
+				set path [lindex $cols 0]
+				set value [join [lrange $cols 2 end] :]
+				if {[string index $path 0] eq "\""} {
+					set path [encoding convertfrom [lindex $path 0]]
+				}
+				regsub {^ } $value {} value
+				set path_attr_cache($attr,$path) $value
+			}
+		}
+		update
+	}
+}
+
 proc get_path_encoding {path} {
 	global gui_encoding
 	set tcl_enc [tcl_encoding $gui_encoding]
-- 
1.6.0.20.g6148bc

^ permalink raw reply related

* Re: [PATCH 3/4] diff: introduce diff.<driver>.binary
From: Matthieu Moy @ 2008-10-13  8:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Johannes Sixt, git
In-Reply-To: <7vk5cddtzh.fsf@gitster.siamese.dyndns.org>

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

> The textconv filter is primarily for humans to be able to view the diff,
> as far as I understand it, but what would this facility do to the patch
> exchange workflow?  There needs either one or both of the following:
>
>  - A command line option to Porcelains to override textconv so that an
>    applicable binary diff can be obtained upon request (or format-patch
>    always disables textconv); and/or

To me (and others in this thread AIUI), the textconv should only
replace the (frustrating) "binary files differ" in porcelain.

>  - You teach git-apply to use a reverse transformation of textconv, so
>    that it does, upon reception of a textconv diff:
>
>    (1) pass existing preimage through textconv;
>    (2) apply the patch;
>    (3) convert the result back to binary.

I can imagine corner-case scenarios where this would be applicable and
somehow usefull (like: modifying _only_ the EXIF tags on one end, and
expect to be able to view and apply the tags modification on the other
end), but I don't think that's something important to worry about. As
other said, it would be really hard to set up for users, with little
benefits.

One possibility that would be simpler to set up would be to include
both the binary diff and the textconved diff in format-patch, and have
"git apply" just verify that the textconv actually matches the binary
diff after applying it. But that reduces the security issue without
really solving it: one could very well send a binary patch that
changes both the image and the exiftags, and claim it only changes the
tags.

In short: agreed with Peff ;-).

-- 
Matthieu

^ permalink raw reply

* Re: User Authentication ?
From: Jakub Narebski @ 2008-10-13  8:23 UTC (permalink / raw)
  To: Neshama Parhoti; +Cc: git
In-Reply-To: <912ec82a0810130109k56e2e976kd00678e2ca3bc558@mail.gmail.com>

Neshama Parhoti wrote:

> On Sat, Oct 11, 2008 at 8:28 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>> "Neshama Parhoti" <pneshama@gmail.com> writes:
>>
>>> I want to setup a git server on the web but I need user
>>> authentication.
>>> I really don't want to give ssh logins for people who I just want to
>>> be able to access my repository...
>>
>> First, you can always set git-shell as shell for those git only
>> accounts. [...]
> 
> Thank you ! That should really be good. Any chance you are aware of
> documentation or further guidance about how to set this up ?

I have never had the need, but for Gitosis the seminal reference is
I think
  "Hosting Git repositories, The Easy (and Secure) Way"
  http://scie.nti.st/2007/11/14/hosting-git-repositories-the-easy-and-secure-way

-- 
Jakub Narebski
Poland

^ 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