threads / discuss / 20037

What's cooking in git.git (Jul 2009, #01; Mon, 06)

Subject: What's cooking in git.git (Jul 2009, #01; Mon, 06)

## tl;dr

20 messages between Jul 6, 2009 and Jul 10, 2009.

replies: 19people: 12as markdown or json

Junio C Hamano· Jul 6, 2009, 18:32 UTC · lore

Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the branches, but I am still holding onto them.

The topics list the commits in reverse chronological order. The topics meant to be merged to the maintenance series have "maint-" in their names.

It has been relatively quiet for the past few weeks. The 'next' branch is getting quite thin, and it would be a good time to declare -rc0. I'll do so by my Wednesday.

---------------------------------------------------------------- [New Topics]

* ld/push-porcelain-output-format (Mon Jun 22 21:10:01 2009 -0400) 1 commit
 + add --porcelain option to git-push
* js/run-command-updates (Sat Jul 4 21:26:43 2009 +0200) 7 commits
 - receive-pack: remove unnecessary run_status report
 - run_command: report failure to execute the program, but optionally
   don't
 - run_command: encode deadly signal number in the return value
 - run_command: report system call errors instead of returning error
   codes
 - run_command: return exit code as positive value
 - MinGW: simplify waitpid() emulation macros
 - MinGW: truncate exit()'s argument to lowest 8 bits

A few replacement/squash updates came in before it hit 'pu'; this should be the latest version.

* cc/sequencer-rebase-i (Fri Jun 26 23:08:46 2009 +0200) 4 commits
 - rebase -i: use "git sequencer--helper --make-patch"
 - sequencer: free memory used in "make_patch" function
 - sequencer: add "make_patch" function to save a patch
 - sequencer: add "builtin-sequencer--helper.c"
* ae/maint-mailinfo-rm-only-one-patch-marker (Mon Jun 29 11:55:51 2009 +0200) 1 commit
 - mailinfo: Remove only one set of square brackets

The change needed to the test vector shows the extent of the damage this change may cause in the real world. A handcrafted "Subject: [area] [PATCH] title" will be turned into "[PATCH] title".

* rs/grep-p (Thu Jul 2 00:06:34 2009 +0200) 7 commits
 + grep: simplify -p output
 + grep -p: support user defined regular expressions
 + grep: add option -p/--show-function
 + grep: handle pre context lines on demand
 + grep: print context hunk marks between files
 + grep: move context hunk mark handling into show_line()
 + userdiff: add xdiff_clear_find_func()

---------------------------------------------------------------- [Graduated to "master"]

* cf/maint-remote-uploadpack-useconfig-fix (Thu Jun 25 17:21:35 2009 -0400) 1 commit
 + git-remote: fix missing .uploadpack usage for show command
* sb/show-ref-parse-options (Sat Jun 20 21:40:46 2009 -0700) 1 commit
 + show-ref: migrate to parse-options
* ne/maint-1.6.0-diff-tree-t-r-show-directory (Sat Jun 13 17:06:09 2009 -0700) 1 commit
 + diff-tree -r -t: include added/removed directories in the output

This changes the output from "diff-tree -r -t"; it brings more consistency to it, but it is a change and could break scripts.

* uk/rev-parse-parse-opt (Sun Jun 14 01:58:43 2009 +0200) 2 commits
 + parse-opt: make PARSE_OPT_STOP_AT_NON_OPTION available to git rev-
   parse
 + more tests for git rev-parse --parse-opt
* js/daemon-log (Sun Jun 21 23:16:09 2009 +0200) 3 commits
 + receive-pack: do not send error details to the client
 + upload-pack: squelch progress indicator if client cannot see it
 + daemon: send stderr of service programs to the syslog
* sb/quiet-porcelains (Wed Jun 17 18:07:37 2009 -0700) 6 commits
 + stash: teach quiet option
 + am, rebase: teach quiet option
 + submodule, repack: migrate to git-sh-setup's say()
 + git-sh-setup: introduce say() for quiet options
 + am: suppress apply errors when using 3-way
 + t4150: test applying with a newline in subject
* jk/use-our-regexp (Fri Jun 19 10:10:39 2009 -0500) 3 commits
 + Makefile: Solaris needs HAVE_ALLOCA_H for alloca()
 + Makefile: use compat regex on Solaris
 + Makefile: refactor regex compat support
* cb/maint-fetch-refspec-wo-dst (Wed Jun 17 15:38:36 2009 +0200) 1 commit
 - fetch: do not create ref from empty name
* cc/bisect (Sat Jun 13 13:11:02 2009 +0200) 2 commits
 + Documentation: remove warning saying that "git bisect skip" may
   slow bisection
 + bisect: use a PRNG with a bias when skipping away from untestable
   commits
* tr/die_errno (Sat Jun 27 17:58:47 2009 +0200) 4 commits
 - Use die_errno() instead of die() when checking syscalls
 - Convert existing die(..., strerror(errno)) to die_errno()
 - die_errno(): double % in strerror() output just in case
 - Introduce die_errno() that appends strerror(errno) to die()

I didn't check the individual conversion from die() to die_errno() in this latest round; comments?

* gb/am-foreign (Wed May 27 11:25:19 2009 +0200) 4 commits
 - git-am: refactor 'cleaning up and aborting'
 - git-am foreign patch support: StGIT support
 - git-am foreign patch support: autodetect some patch formats
 - git-am foreign patch support: introduce patch_format
Will be in 'next' shortly.

---------------------------------------------------------------- [Stalled and may need help and prodding to go forward]

* ml/http (Wed May 27 23:16:03 2009 -0400) 2 commits
 - http.c: add http.sslCertPasswordProtected option
 - http.c: prompt for SSL client certificate password

I've rewritten these two to (1) move the #ifdef out of the main codepath, and (2) use configuration/environment to make the misfeature of always asking for a passphrase even a key/cert is unencrypted optional. I tried to be careful but extra sets of eyeballs would be nice to check the result.

Nobody seems to be jumping up-and-down asking for this or helping to push this forward. Perhaps it's time to drop it?

* jh/notes (Sat May 16 13:44:17 2009 +0200) 5 commits
 - Teach "-m <msg>" and "-F <file>" to "git notes edit"
 - Add an expensive test for git-notes
 - Speed up git notes lookup
 - Add a script to edit/inspect notes
 - Introduce commit notes

Dscho asked about the performance implications of this; I do not think I saw any progress on that yet...

* lt/read-directory (Fri May 15 12:01:29 2009 -0700) 3 commits
 - Add initial support for pathname conversion to UTF-8
 - read_directory(): infrastructure for pathname character set
   conversion
 - Add 'fill_directory()' helper function for directory traversal

Before adding the real "conversion", this needs a few real fixups, I think. For example there is one hardcoded array that is used without bounds check.

* ar/maint-1.6.2-merge-recursive-d-f (Mon May 11 21:25:36 2009 +0200) 2 commits
 - Fix for a merge where a branch has an F->D transition
 - Add a reminder test case for a merge with F/D transition

Although the reported breakage is covered with the patch, Alex feels the solution unsatisfactory. Cleaning up D/F conflict handling in merge-recursive may be long overdue but seems to be a hard problem.

* ps/blame (Thu Mar 12 21:30:03 2009 +1100) 1 commit
 - blame.c: start libifying the blame infrastructure
A few minor point remains in this initial one.
* jc/log-tz (Tue Mar 3 00:45:37 2009 -0800) 1 commit
 - Allow --date=local --date=other-format to work as expected

The one I posted had a few corner-case bugs that was caught with the test suite; this one has them fixed. People did not like the UI so it is kept out of 'next'

* jc/merge-convert (Mon Jan 26 16:45:01 2009 -0800) 1 commit
 - git-merge-file: allow converting the results for the work tree
This is a feature waiting for a user.

We did not give scripted Porcelains a way to say "this temporary file I am using for merging is for this path, so use the core.autocrlf and attributes rules for that final path". Instead, merge-file simply wrote out the data in the canonical repository representation.

rerere has the same issue, but it is a lot worse. It reads the three files (preimage, postimage and thisimage) from the work tree in the work tree representation, merges them without converting them to the canonical representation first but inserts the conflict markers with the canonical representation and writes the resulting mess out. It needs to be fixed to read with convert_to_git(), merge them while they are still in the canonical representation and possibly add conflict markers, and then write the results out after convert_to_working_tree(). It also needs to write in binary mode as well.

* db/foreign-scm (Tue Mar 24 23:04:12 2009 -0400) 3 commits
 - Add option for using a foreign VCS
 - Document details of transport function APIs
 - Allow late reporting of fetched hashes
* hv/cvsps-tests (Sun Apr 5 01:40:50 2009 -0700) 8 commits
 - t/t9600: remove exit after test_done
 - cvsimport: extend testcase about patchset order to contain
   branches
 - cvsimport: add test illustrating a bug in cvsps
 - Add a test of "git cvsimport"'s handling of tags and branches
 - Add some tests of git-cvsimport's handling of vendor branches
 - Test contents of entire cvsimported "master" tree contents
 - Use CVS's -f option if available (ignore user's ~/.cvsrc file)
 - Start a library for cvsimport-related tests

---------------------------------------------------------------- [Actively cooking]

* gb/gitweb-avatar (Tue Jun 30 00:00:54 2009 +0200) 7 commits
 - gitweb: add empty alt text to avatar img
 - gitweb: picon avatar provider
 - gitweb: gravatar url cache
 - gitweb: (gr)avatar support
 - gitweb: use git_print_authorship_rows in 'tag' view too
 - gitweb: uniform author info for commit and commitdiff
 - gitweb: refactor author name insertion

This should be the latest one posted to the list, and I think it is reasonable, and Jakub seemed to concur. Will be in 'next'

* en/fast-export (Thu Jun 25 22:48:33 2009 -0600) 7 commits
 - fast-export: Document the fact that git-rev-list arguments are
   accepted
 - Add new fast-export testcases
 - fast-export: Add a --tag-of-filtered-object option for newly
   dangling tags
 - fast-export: Do parent rewriting to avoid dropping relevant
   commits
 - fast-export: Make sure we show actual ref names instead of
   "(null)"
 - fast-export: Omit tags that tag trees
 - fast-export: Set revs.topo_order before calling setup_revisions
Shawn?  Dscho?
* jc/diff-whitespace-only-status (Sat May 23 01:15:35 2009 -0700) 2 commits
 - diff: Rename QUIET internal option to QUICK
 - diff: change semantics of "ignore whitespace" options

I am not sure if it should wait for a major version bump but this is a good semantics change. Perhaps merge to 'next' soonish, but I am undecided. Comments?

For the following three series, I have not managed to convince myself if these changes have real-world needs.

* sb/read-tree (Thu Jun 25 22:14:10 2009 -0700) 2 commits
 - read-tree: migrate to parse-options
 - read-tree: convert unhelpful usage()'s to helpful die()'s
* ne/futz-upload-pack (Wed Jun 10 01:50:18 2009 +0200) 1 commit
 - Shift object enumeration out of upload-pack
* cc/replace (Wed May 27 07:14:09 2009 +0200) 14 commits
 - t6050: check pushing something based on a replaced commit
 - Documentation: add documentation for "git replace"
 - Add git-replace to .gitignore
 - builtin-replace: use "usage_msg_opt" to give better error messages
 - parse-options: add new function "usage_msg_opt"
 - builtin-replace: teach "git replace" to actually replace
 - Add new "git replace" command
 - environment: add global variable to disable replacement
 - mktag: call "check_sha1_signature" with the replacement sha1
 - replace_object: add a test case
 - object: call "check_sha1_signature" with the replacement sha1
 - sha1_file: add a "read_sha1_file_repl" function
 - replace_object: add mechanism to replace objects found in
   "refs/replace/"
 - refs: add a "for_each_replace_ref" function

---------------------------------------------------------------- [On Hold]

* jc/deny-delete-current-1.7.0 (Mon Feb 9 00:19:46 2009 -0800) 1 commit
 - receive-pack: default receive.denyDeleteCurrent to refuse
* jc/refuse-push-to-current-1.7.0 (Wed Feb 11 02:28:03 2009 -0800) 1 commit
 - Refuse updating the current branch in a non-bare repository via
   push

These are for 1.7.0, but the messages when they trigger together may need to be rethought.

Marcus Camen· Jul 6, 2009, 20:29 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

On Montag 06 Juli 2009, Junio C Hamano wrote:
Show 12 quoted lines
> * ml/http (Wed May 27 23:16:03 2009 -0400) 2 commits
>  - http.c: add http.sslCertPasswordProtected option
>  - http.c: prompt for SSL client certificate password
>
> I've rewritten these two to (1) move the #ifdef out of the main
> codepath, and (2) use configuration/environment to make the misfeature
> of always asking for a passphrase even a key/cert is unencrypted
> optional.  I tried to be careful but extra sets of eyeballs would be
> nice to check the result.
>
> Nobody seems to be jumping up-and-down asking for this or helping to
> push this forward.  Perhaps it's time to drop it?
/me jumping up-and-down

This fix is crucial for corporate environments where HTTPS is the only way to access GIT repositories outside the firewall.

I verified the patch and everything works as expected.

-- Marcus

Junio C Hamano· Jul 6, 2009, 21:38 UTC · re: Marcus Camen · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

Marcus Camen <mcamen@mcamen.de> writes:
Show 20 quoted lines
> On Montag 06 Juli 2009, Junio C Hamano wrote:
>> * ml/http (Wed May 27 23:16:03 2009 -0400) 2 commits
>>  - http.c: add http.sslCertPasswordProtected option
>>  - http.c: prompt for SSL client certificate password
>>
>> I've rewritten these two to (1) move the #ifdef out of the main
>> codepath, and (2) use configuration/environment to make the misfeature
>> of always asking for a passphrase even a key/cert is unencrypted
>> optional.  I tried to be careful but extra sets of eyeballs would be
>> nice to check the result.
>>
>> Nobody seems to be jumping up-and-down asking for this or helping to
>> push this forward.  Perhaps it's time to drop it?
>
> /me jumping up-and-down
>
> This fix is crucial for corporate environments where HTTPS is the only way 
> to access GIT repositories outside the firewall.
>
> I verified the patch and everything works as expected.
Thanks.
What did you exactly mean by "everything"?
 - On a protected key/cert, with configuration, it asks the question once.
 - On an unprotected key/cert, without configuration, it never asks the
   question.
 - On an unprotected key/cert, with configuration, it asks an useless
   question but it does so only once.

You tested all of the above? I am not demanding you to test all of these, but we need to make sure at least somebody did. Especially because it would be a regression if the second one does not.

Marcus Camen· Jul 6, 2009, 22:03 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

Show 8 quoted lines
>  - On a protected key/cert, with configuration, it asks the question
> once.
>  - On an unprotected key/cert, without configuration, it never asks the
>    question.
>  - On an unprotected key/cert, with configuration, it asks an useless
>    question but it does so only once.
>
> You tested all of the above?
Yes, all three tests run exactly as you described.
In addition
   - On a protected key/cert, without configuration
GIT shows the same behaviour as without the patch.

I checked using http.sslCertPasswordProtected and also GIT_SSL_CERT_PASSWORD_PROTECTED. curl is 7.19.5

Just let me know if you need more checks.
Marcus
Junio C Hamano· Jul 6, 2009, 22:34 UTC · re: Marcus Camen · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

Marcus Camen <mcamen@mcamen.de> writes:
Show 10 quoted lines
>>  - On a protected key/cert, with configuration, it asks the question
>> once.
>>  - On an unprotected key/cert, without configuration, it never asks the
>>    question.
>>  - On an unprotected key/cert, with configuration, it asks an useless
>>    question but it does so only once.
>>
>> You tested all of the above?
>
> Yes, all three tests run exactly as you described.
Wonderful; thanks.
Jakub Narebski· Jul 6, 2009, 23:42 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

Junio C Hamano <gitster@pobox.com> writes:
Show 13 quoted lines
> [Actively cooking]
> 
> * gb/gitweb-avatar (Tue Jun 30 00:00:54 2009 +0200) 7 commits
>  - gitweb: add empty alt text to avatar img
>  - gitweb: picon avatar provider
>  - gitweb: gravatar url cache
>  - gitweb: (gr)avatar support
>  - gitweb: use git_print_authorship_rows in 'tag' view too
>  - gitweb: uniform author info for commit and commitdiff
>  - gitweb: refactor author name insertion
> 
> This should be the latest one posted to the list, and I think it is
> reasonable, and Jakub seemed to concur.  Will be in 'next'
I concur.

It is now, after many iterations, well written and nicely crafted series, IMVHO.

-- 
Jakub Narebski
Poland
ShadeHawk on #git
Mark Lodato· Jul 7, 2009, 02:18 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

On Mon, Jul 6, 2009 at 2:32 PM, Junio C Hamano<gitster@pobox.com> wrote:
Show 13 quoted lines
> [Stalled and may need help and prodding to go forward]
>
> * ml/http (Wed May 27 23:16:03 2009 -0400) 2 commits
>  - http.c: add http.sslCertPasswordProtected option
>  - http.c: prompt for SSL client certificate password
>
> I've rewritten these two to (1) move the #ifdef out of the main codepath,
> and (2) use configuration/environment to make the misfeature of always
> asking for a passphrase even a key/cert is unencrypted optional.  I tried
> to be careful but extra sets of eyeballs would be nice to check the result.
>
> Nobody seems to be jumping up-and-down asking for this or helping to push
> this forward.  Perhaps it's time to drop it?

Sorry for the lack of updates. After hearing feedback, the consensus seemed to be that detection of the certificate's encryption (above) and file type (other patch, not in git.git) should be done automatically, that is, without user configuration. I agree, but neither can be done without great difficulty outside of libcurl. Therefore, I have started implement the autodetection of both, as well as the password caching, directly in libcurl. If my work, once completed, is accepted by the libcurl folks, then there would be no need for the above, and we should recommend upgrading libcurl for those who want to use client-side certificates.

However, in the interim, and for users with earlier libcurl versions (and especially if my libcurl patch is never accepted), it might be nice to still have the above commits. They are unobtrusive - the patches are small, and they do not affect users who do not enable the option - yet they drastically improve the experience for those using password-protected client-side certificates.

Anyway, I am still very interested in getting proper client-side certificate support in git, and I am glad to see Marcus is as well. Ultimately, I think the libcurl solution is the proper way to go, but this patch series might still be good to include in git. The downside is that it adds extra crap to the git-config man page and it does increase the code size a little.

-- Mark

Jeff King· Jul 7, 2009, 21:11 UTC · re: Mark Lodato · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

On Mon, Jul 06, 2009 at 10:18:03PM -0400, Mark Lodato wrote:
Show 22 quoted lines
> > * ml/http (Wed May 27 23:16:03 2009 -0400) 2 commits
> >  - http.c: add http.sslCertPasswordProtected option
> >  - http.c: prompt for SSL client certificate password
> [...]
> 
> Sorry for the lack of updates.  After hearing feedback, the consensus
> seemed to be that detection of the certificate's encryption (above)
> and file type (other patch, not in git.git) should be done
> automatically, that is, without user configuration.  I agree, but
> neither can be done without great difficulty outside of libcurl.
> Therefore, I have started implement the autodetection of both, as well
> as the password caching, directly in libcurl.  If my work, once
> completed, is accepted by the libcurl folks, then there would be no
> need for the above, and we should recommend upgrading libcurl for
> those who want to use client-side certificates.
> 
> However, in the interim, and for users with earlier libcurl versions
> (and especially if my libcurl patch is never accepted), it might be
> nice to still have the above commits.  They are unobtrusive - the
> patches are small, and they do not affect users who do not enable the
> option - yet they drastically improve the experience for those using
> password-protected client-side certificates.

Yes, even if you get patches into libcurl, we will be supporting libcurl without this feature for some time. So I think the right upgrade path is:

  1. add sslCertPasswordProtected as a bool config option now, defaulting
     to false (i.e., your patches)
  2. libcurl grows new auto-detect feature
  3. When the new feature is available at build-time, turn
     sslCertPasswordProtected into a tri-state yes/no/auto, defaulting to
     "auto". For older libcurl, setting it to "auto" should probably
     generate an error (or perhaps simply default to "no").

IOW, whether the libcurl auto-detection feature happens or not, your patches are the right first step (and if "not", then they are the final step :) ).

-Peff
Johannes Sixt· Jul 7, 2009, 06:30 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

Junio C Hamano schrieb:
> * ne/futz-upload-pack (Wed Jun 10 01:50:18 2009 +0200) 1 commit
>  - Shift object enumeration out of upload-pack

I'm interested in this one because it is a step towards improved behavior of upload-pack on Windows if the repository is corrupted[*]. This patch covers the common case where shallow clones are out of the game, but it is not ready for prime time until its implementation is complete. IIUC, this should be a fall-out of a GSoC project. Until then I include it in my git.

[*] One test case in t5530 still fails on Windows, because for some reason errors are not reported correctly. It has to do with the rev-list being run in a thread and that thread die()s.

-- Hannes
Linus Torvalds· Jul 7, 2009, 19:17 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

On Mon, 6 Jul 2009, Junio C Hamano wrote:
Show 10 quoted lines
> 
> * lt/read-directory (Fri May 15 12:01:29 2009 -0700) 3 commits
>  - Add initial support for pathname conversion to UTF-8
>  - read_directory(): infrastructure for pathname character set
>    conversion
>  - Add 'fill_directory()' helper function for directory traversal
> 
> Before adding the real "conversion", this needs a few real fixups, I
> think.  For example there is one hardcoded array that is used without
> bounds check.

Hmm. I'm not sure what array you're talking about (the newpath/newbase ones? We do protect against PATH_MAX, it's just that we protect against it in the "previous iteration").

The bigger issue, though, is that I spent half a day looking more at this series last Thursday, and I've got some improvements, but getting "all the way" turns out to be really quite painful.

Why?

We have a _lot_ of code that does "lstat()" on pathnames, and it all basically uses the internal git representation of the pathname. In particular, we do this a lot for index lookups, but it's true in other cases too (example: things like tree merging, where we check whether a file exists in the working tree).

To test this all out, I actually fleshed out the patches to the point where I could do

	[core]
		PathEncoding = Latin1

and actually have the working tree use Latin1 encoding, and convert internally in git to UTF-8, and have a working "git add ."

However, "git add ." was just about the only thing that I made do the right thing. Even doing a simple "git diff" afterwards would then show the file as deleted, because the UTF-8 version of the file (that the index contained) didn't exist in the filesystem. I fixed that with a hack, but it basically turns out to be pretty damn ugly, and there's a _lot_ of those places.

So, the question is, "What now?"
There's a few alternatives:
(a) don't do any of this crap at all. What git does right now works fairly
    well for most people. Instead, perhaps worry about just the crazy 
    case-insensitive filesystems, which are a totally separate issue.
    End result: git will always have problems with the crazy NFD format
    that OS X uses. Mixing git archives across OS X and other saner
    operating systems (and in this context, Windows really does count as
    "saner" - it really is OS X that is braindamaged!) will be painful if
    you have odd characters in your working tree.
    This is the simplest approach, of course. The case-insensitivity is 
    still not trivial, but we could work on it, and it really is a 
    different problem (and has none of the "if you look the file up with a 
    converted name, you cannot see it" issues that the Latin1<->UTF8 
    example had).
(b) Forget about the general case (like Latin1) that needs two-way 
    conversion. Just worry about OS X being crazy, and do the NFD->NFC 
    translation, which only needs to be done one way (because OS X will
    still accept and recognize NFC characters, so the "converted" path is 
    still seen as valid by 'lstat()' and friends).
    This is very much just a special case of handling filesystems that are 
    UTF-8, but are confused about what "equivalent" and "identical" means, 
    and where the filesystem designer was a moron on some seriously crazy 
    drugs, and thought that equivalence means identity, and thought that 
    NFD is a sane form to expose.
    This is a much simpler case than the general approach. I don't have OS 
    X to test with, though, and so far it hasn't appeared that any OS X 
    people really care about to actually implement it. So I can fix up my 
    series to a certain point, but will never be able to really do the 
    final testing and tuning. At least with the full "treat filesystem as 
    Latin1 encoding", I could _test_ it.
(c) Try to bite the bullet. I can do this, but it really is going to be a 
    _very_ invasive patch-series, and it will probably involve some nasty 
    changes to the index format (for performance, we'll likely have to 
    change the index to have _both_ the "git filename", and the 
    "filesystem filename" in it).
    This was what I wanted to do, and it's what you'd need to do if you do 
    things like Latin1 filesystem trees or ones where pathnames are done 
    with shift-JIS encoding or if we want to actually use the (crazy) 
    native Windows UCS filesystem accessors or whatever.
    But I have to admit that after looking at the pain, I'm not at all 
    convinced it's worth it. Do we ever want to say "git supports 
    filesystems with shift-JIS encoding"? Do people really care deeply 
    enough about non-utf filesystems that they'd be willing to live with a 
    _lot_ of pretty nasty complexity, and some real performance overhead?

I have to say, even with plain UTF-8, git isn't really a pleasure to use. While I did my Latin1 test, I used filenames like "åäö" (the three extra Finnish/Swedish characters), and if you do this

	mkdir test-repo
	cd test-repo
	git init
	echo testfile > åäö
	git add .
	git ls-files

the end result is not actually really usable. We quote it to a binary mess, rather than showing "åäö". Our pathname quoting is trying to be safe, which is good, but it does mean that right now, odd characters aren't very friendly even _if_ you are using a sane filesystem, and all plain NFC utf-8.

So right now, my personal opinion is:
 - let's just face the fact that the only sane filename representation is 
   NFC UTF-8. Show filenames as UTF-8 when possible, rather than quoting 
   them.
 - Do case (b) above: add support for converting NFD -> NFC at readdir() 
   time, so that OS X people can use UTF-8 sanely. 
 - add a "binary encoding" mode to filesystems that actually use Latin1, 
   just so that if people use Latin1 or Shift-JIS filesystem encodings, we 
   promise that we'll never munge those kinds of names. 
 - Maybe we'd make the "binary encoding" (which is effectively existing 
   git behavior) be the default on non-OSX platforms.

but that's just my gut feel from trying to weigh the costs of trying to do something more involved against the costs of OS X support and just letting crazy encodings exist in their own little worlds. So a development group that uses Shift-JIS (or Latin1) would be able to work internally with git that way, but would not be able to sanely work with the world at large that uses UTF-8.

		Linus
Alex Riesen· Jul 7, 2009, 19:57 UTC · re: Linus Torvalds · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

On Tue, Jul 7, 2009 at 21:17, Linus Torvalds<torvalds@linux-foundation.org> wrote:

Show 22 quoted lines
> So right now, my personal opinion is:
>
>  - let's just face the fact that the only sane filename representation is
>   NFC UTF-8. Show filenames as UTF-8 when possible, rather than quoting
>   them.
>
>  - Do case (b) above: add support for converting NFD -> NFC at readdir()
>   time, so that OS X people can use UTF-8 sanely.
>
>  - add a "binary encoding" mode to filesystems that actually use Latin1,
>   just so that if people use Latin1 or Shift-JIS filesystem encodings, we
>   promise that we'll never munge those kinds of names.
>
>  - Maybe we'd make the "binary encoding" (which is effectively existing
>   git behavior) be the default on non-OSX platforms.
>
> but that's just my gut feel from trying to weigh the costs of trying to do
> something more involved against the costs of OS X support and just letting
> crazy encodings exist in their own little worlds. So a development group
> that uses Shift-JIS (or Latin1) would be able to work internally with git
> that way, but would not be able to sanely work with the world at large
> that uses UTF-8.

Maybe we could at least let the user save the encoding of file names in the tree objects somehow?

Linus Torvalds· Jul 7, 2009, 22:13 UTC · re: Alex Riesen · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

On Tue, 7 Jul 2009, Alex Riesen wrote:
> 
> Maybe we could at least let the user save the encoding of file names
> in the tree objects somehow?

There's no place to really sanely save it in a tree, nor is there really even any way to figure the right encoding out. In fact, one of the problems with non-utf encodings is that in theory people could literally be mixing different encodings in one tree (imagine test-suites etc).

There's a reason why the only _sane_ model is to use an encoding that is universal.

So we could in theory save an encoding in the commit, but quite frankly, it's unlikely that we could use it sanely. Nobody is ever going to write a sane merge that will merge across different encodings. So realistically, you're going to have a single encoding not per tree, or per commit, but for the whole project.

And then when you get fed up with Latin1 or Shift-JIS or whatever crazy sh*t people are still using, you use fast-export + script + fast-import, and change the encoding for the whole repository that way. Exactly because doing it at a smaller granularity is a total pain in the *ss.

			Linus
Johannes Schindelin· Jul 7, 2009, 20:08 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

Hi,
On Mon, 6 Jul 2009, Junio C Hamano wrote:
Show 9 quoted lines
> * jh/notes (Sat May 16 13:44:17 2009 +0200) 5 commits
>  - Teach "-m <msg>" and "-F <file>" to "git notes edit"
>  - Add an expensive test for git-notes
>  - Speed up git notes lookup
>  - Add a script to edit/inspect notes
>  - Introduce commit notes
> 
> Dscho asked about the performance implications of this; I do not think I 
> saw any progress on that yet...
Neither did I.
Show 14 quoted lines
> * en/fast-export (Thu Jun 25 22:48:33 2009 -0600) 7 commits
>  - fast-export: Document the fact that git-rev-list arguments are
>    accepted
>  - Add new fast-export testcases
>  - fast-export: Add a --tag-of-filtered-object option for newly
>    dangling tags
>  - fast-export: Do parent rewriting to avoid dropping relevant
>    commits
>  - fast-export: Make sure we show actual ref names instead of
>    "(null)"
>  - fast-export: Omit tags that tag trees
>  - fast-export: Set revs.topo_order before calling setup_revisions
> 
> Shawn?  Dscho?

Sorry, today I was still jet-lagged, and I do not trust myself in such conditions at all. Will take another look tomorrow (what I saw looked okay to me, but I did not have time to look closely).

Ciao, Dscho

Shawn O. Pearce· Jul 7, 2009, 20:13 UTC · re: Johannes Schindelin · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 13 quoted lines
> On Mon, 6 Jul 2009, Junio C Hamano wrote:
> 
> > * jh/notes (Sat May 16 13:44:17 2009 +0200) 5 commits
> >  - Teach "-m <msg>" and "-F <file>" to "git notes edit"
> >  - Add an expensive test for git-notes
> >  - Speed up git notes lookup
> >  - Add a script to edit/inspect notes
> >  - Introduce commit notes
> > 
> > Dscho asked about the performance implications of this; I do not think I 
> > saw any progress on that yet...
> 
> Neither did I.

I was thinking about this the other day. We could use a hash of the commit timestamp as the top level directory. E.g. if we take the commit time of the commit and convert it to a date string, we could make the note path e.g.:

  YYYY/MM/COMMITSHA1

The advantage is we only need to scan and hash the subtrees for the range of commits we are currently producing output for. As we go further back in time, we can evict entries for newer dates and hash the older dates.

-- 
Shawn.
Junio C Hamano· Jul 7, 2009, 22:19 UTC · re: Shawn O. Pearce · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

"Shawn O. Pearce" <spearce@spearce.org> writes:
Show 26 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> On Mon, 6 Jul 2009, Junio C Hamano wrote:
>> 
>> > * jh/notes (Sat May 16 13:44:17 2009 +0200) 5 commits
>> >  - Teach "-m <msg>" and "-F <file>" to "git notes edit"
>> >  - Add an expensive test for git-notes
>> >  - Speed up git notes lookup
>> >  - Add a script to edit/inspect notes
>> >  - Introduce commit notes
>> > 
>> > Dscho asked about the performance implications of this; I do not think I 
>> > saw any progress on that yet...
>> 
>> Neither did I.
>
> I was thinking about this the other day.  We could use a hash of
> the commit timestamp as the top level directory.  E.g. if we take
> the commit time of the commit and convert it to a date string,
> we could make the note path e.g.:
>
>   YYYY/MM/COMMITSHA1
>
> The advantage is we only need to scan and hash the subtrees for
> the range of commits we are currently producing output for.  As we
> go further back in time, we can evict entries for newer dates and
> hash the older dates.

Is the idea to make the tree object we need to scan for that particular SHA-1 hash smaller?

If so, I am not sure how it would help over another approach of say taking the first four hexdigits from the SHA-1 to use as the initial fan-out YYYY, then two hexdigits for the secondary fan-out MM.

But probably I am missing something.
Besides, trees and blobs cannot be annotated with that approach.
Shawn O. Pearce· Jul 7, 2009, 22:28 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

Junio C Hamano <gitster@pobox.com> wrote:
Show 13 quoted lines
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> >> 
> >> > * jh/notes (Sat May 16 13:44:17 2009 +0200) 5 commits
> >
> > I was thinking about this the other day.  We could use a hash of
> > the commit timestamp as the top level directory.  E.g. if we take
> > the commit time of the commit and convert it to a date string,
> > we could make the note path e.g.:
> >
> >   YYYY/MM/COMMITSHA1
> 
> Is the idea to make the tree object we need to scan for that particular
> SHA-1 hash smaller?

No, the idea was to avoid needing to create a massive hash of all commit notes just to answer `git log -10` on the current branch. I remember that was a concern last time we were talking about this. By putting the notes under a timestamped path we can scan only a small percentage of the notes before we have sufficient data to output the first few commits.

> If so, I am not sure how it would help over another approach of say taking
> the first four hexdigits from the SHA-1 to use as the initial fan-out
> YYYY, then two hexdigits for the secondary fan-out MM.
See above, the idea is to avoid scanning all notes at once on
startup.  SHA-1 is bad at this as a fanout because it is too good
at uniform distribution of the names.
 
> But probably I am missing something.
> 
> Besides, trees and blobs cannot be annotated with that approach.
True.  But I didn't realize that was a goal.  :-|
-- 
Shawn.
Johannes Schindelin· Jul 8, 2009, 13:42 UTC · re: Shawn O. Pearce · lore

notes, was Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

Hi,
On Tue, 7 Jul 2009, Shawn O. Pearce wrote:
Show 21 quoted lines
> Junio C Hamano <gitster@pobox.com> wrote:
> > "Shawn O. Pearce" <spearce@spearce.org> writes:
> > >> 
> > >> > * jh/notes (Sat May 16 13:44:17 2009 +0200) 5 commits
> > >
> > > I was thinking about this the other day.  We could use a hash of the 
> > > commit timestamp as the top level directory.  E.g. if we take the 
> > > commit time of the commit and convert it to a date string, we could 
> > > make the note path e.g.:
> > >
> > >   YYYY/MM/COMMITSHA1
> > 
> > Is the idea to make the tree object we need to scan for that 
> > particular SHA-1 hash smaller?
> 
> No, the idea was to avoid needing to create a massive hash of all
> commit notes just to answer `git log -10` on the current branch.
> I remember that was a concern last time we were talking about this.
> By putting the notes under a timestamped path we can scan only a
> small percentage of the notes before we have sufficient data to
> output the first few commits.

The problem is that you end up with possibly _very_ large root trees in the notes, and the whole idea was to reduce the root tree, and load the subtrees only on demand. That way, outputting a couple of commits (or a single one) is still cheap.

To recapitulate mugwump's idea: allow not only blobs in the root tree of the notes, but also tree objects. That allows for fan-out -- if you want it.

Example:

Commit 0123456789abcdef0123456789abcdef01234567 can be in refs/notes:0123456789abcdef0123456789abcdef01234567 or in refs/notes:01/23456789abcdef0123456789abcdef01234567 or in refs/notes:01/23/456789abcdef0123456789abcdef01234567 or in

My idea was to let shorter paths (in terms of characters used) precedence (and longer prefixes). There was also the idea to always show all of them, but that would not appeal to me from a performance angle.

Show 7 quoted lines
> > If so, I am not sure how it would help over another approach of say 
> > taking the first four hexdigits from the SHA-1 to use as the initial 
> > fan-out YYYY, then two hexdigits for the secondary fan-out MM.
> 
> See above, the idea is to avoid scanning all notes at once on startup.  
> SHA-1 is bad at this as a fanout because it is too good at uniform 
> distribution of the names.
The problem is the unpacking of the tree object.
> > Besides, trees and blobs cannot be annotated with that approach.
> 
> True.  But I didn't realize that was a goal.  :-|
It would be a nice-to-have, I guess.

Ciao, Dscho

Stephen Boyd· Jul 8, 2009, 05:39 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

Junio C Hamano wrote:
Show 6 quoted lines
> For the following three series, I have not managed to convince myself if
> these changes have real-world needs.
>
> * sb/read-tree (Thu Jun 25 22:14:10 2009 -0700) 2 commits
>  - read-tree: migrate to parse-options
>  - read-tree: convert unhelpful usage()'s to helpful die()'s

I think the first patch is good. I am still thinking about the second one though. Obviously rc is coming so it's not too pressing.

I've done a quick survey of parse-optification of builtins and I see that send-pack, rev-list, and fetch-pack all set bitfields when parsing options. Migrating these will have the same problems. I don't know if there's really any simple answer to it though, because you can't take the address of a bitfield. And I think it's stupid to make the bitfields into full ints just to support parse-options.

Maybe some builtins are just never meant to be migrated. Like update-index and its --cacheinfo option.

Johannes Sixt· Jul 8, 2009, 06:38 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

Junio C Hamano schrieb:
> It has been relatively quiet for the past few weeks.  The 'next' branch is
> getting quite thin, and it would be a good time to declare -rc0.  I'll do
> so by my Wednesday.
...
Show 10 quoted lines
> * js/run-command-updates (Sat Jul 4 21:26:43 2009 +0200) 7 commits
>  - receive-pack: remove unnecessary run_status report
>  - run_command: report failure to execute the program, but optionally
>    don't
>  - run_command: encode deadly signal number in the return value
>  - run_command: report system call errors instead of returning error
>    codes
>  - run_command: return exit code as positive value
>  - MinGW: simplify waitpid() emulation macros
>  - MinGW: truncate exit()'s argument to lowest 8 bits

Please include the first one of this series (truncate exit code) in -rc0; it fixes a bug in git-bisect on Windows.

Thanks, -- Hannes

Christian Couder· Jul 10, 2009, 05:05 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Jul 2009, #01; Mon, 06)

On Monday 06 July 2009, Junio C Hamano wrote:
> For the following three series, I have not managed to convince myself if
> these changes have real-world needs.
[...]
Show 16 quoted lines
> * cc/replace (Wed May 27 07:14:09 2009 +0200) 14 commits
>  - t6050: check pushing something based on a replaced commit
>  - Documentation: add documentation for "git replace"
>  - Add git-replace to .gitignore
>  - builtin-replace: use "usage_msg_opt" to give better error messages
>  - parse-options: add new function "usage_msg_opt"
>  - builtin-replace: teach "git replace" to actually replace
>  - Add new "git replace" command
>  - environment: add global variable to disable replacement
>  - mktag: call "check_sha1_signature" with the replacement sha1
>  - replace_object: add a test case
>  - object: call "check_sha1_signature" with the replacement sha1
>  - sha1_file: add a "read_sha1_file_repl" function
>  - replace_object: add mechanism to replace objects found in
>    "refs/replace/"
>  - refs: add a "for_each_replace_ref" function
Didn't you say before that it would be an improvement over grafts?

By the way, may be it would be an improvement to use one bit in "struct object" to mark replaced objects. It could make it easier to spot them where they could make the code behave strangely.

Anyway I will have a 2 week long vacation starting today, so I won't be able to do much soon.

Best regards, Christian.

← back to recent threads