Git development
 help / color / mirror / Atom feed
* Re: [PATCH 0/9] Bisect skip
From: Shawn O. Pearce @ 2007-10-22  6:02 UTC (permalink / raw)
  To: Christian Couder; +Cc: Junio Hamano, Johannes Schindelin, git
In-Reply-To: <200710220747.28731.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> wrote:
> Here is the "bisect skip" patch series.
> It's just a rename from "dunno" to "skip" compared to the previous "dunno" 
> patch series that was in Shawn's pu branch.
> 
> In fact there is no change in the first 3 patches and trivial changes in the 
> other patches.

Thanks.  I'm reparking this new version in pu tonight.  It is too
late in the morning here for me to go through this any further than
applying and merging.  I'm almost done looking through and testing
the new option parser series and then will be looking at this one
next; probably Tuesday.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] "git help -a" should search all exec_paths and PATH
From: Johannes Sixt @ 2007-10-22  6:06 UTC (permalink / raw)
  To: Scott R Parish; +Cc: git
In-Reply-To: <20071021214846.GI16291@srparish.net>

Scott R Parish schrieb:
> +	if (!dirp || chdir(dir)) {
> +		fchdir(start_dir);

/me dislikes this. Windows doesn't have fchdir().

AFAIKS you are only using this to chdir back to where you started, but is 
this necessary? Aren't we exiting anyway after the command list was printed?

-- Hannes

^ permalink raw reply

* What's in git/spearce.git (stable)
From: Shawn O. Pearce @ 2007-10-22  6:11 UTC (permalink / raw)
  To: git

For those who may be new to the git/spearce.git concept I'm filling
in for Junio and have published a tree on repo.or.cz:

  git://repo.or.cz/git/spearce.git
  http://repo.or.cz/r/git/spearce.git

------

* The 'maint' branch has these fixes since the last announcement.

Alex Bennee (1):
  Ensure we add directories in the correct order

Andrew Clausen (1):
  helpful error message when send-pack finds no refs in common.

Brian Gernhardt (1):
  cvsserver: Use exit 1 instead of die when req_Root fails.

Gerrit Pape (1):
  git-config: print error message if the config file cannot be read

Jeff King (1):
  send-pack: respect '+' on wildcard refspecs

Joakim Tjernlund (1):
  Improve receive-pack error message about funny ref creation

Johannes Schindelin (3):
  Fix setup_git_directory_gently() with relative GIT_DIR &
      GIT_WORK_TREE
  fix filter-branch documentation
  filter-branch: update current branch when rewritten

Julian Phillips (1):
  fast-import: Fix argument order to die in file_change_m

Linus Torvalds (4):
  git-blame shouldn't crash if run in an unmerged tree
  Avoid scary errors about tagged trees/blobs during git-fetch
  Fix directory scanner to correctly ignore files without d_type
  Fix diffcore-break total breakage

Patrick Welche (1):
  Define NI_MAXSERV if not defined by operating system

Ralf Wildenhues (1):
  gitk.txt: Fix markup.

Robert Schiele (1):
  fixing output of non-fast-forward output of post-receive-email

Shawn O. Pearce (16):
  git-gui: Display message box when we cannot find git in $PATH
  git-gui: Handle starting on mapped shares under Cygwin
  git-gui: Ensure .git/info/exclude is honored in Cygwin workdirs
  git-gui: Allow gitk to be started on Cygwin with native Tcl/Tk
  git-gui: Don't crash when starting gitk from a browser session
  Correct typos in release notes for 1.5.3.5
  Avoid 'expr index' on Mac OS X as it isn't supported
  Document additional 1.5.3.5 fixes in release notes
  Yet more 1.5.3.5 fixes mentioned in release notes
  Avoid invoking diff drivers during git-stash
  Further 1.5.3.5 fixes described in release notes
  Paper bag fix diff invocation in 'git stash show'
  git-gui: Correctly report failures from git-write-tree
  git-gui: Handle progress bars from newer gits
  git-gui: Don't display CR within console windows
  Describe more 1.5.3.5 fixes in release notes

Simon Sasburg (1):
  git-gui: Avoid using bold text in entire gui for some fonts

Steffen Prohaska (2):
  git-gui: accept versions containing text annotations, like
      1.5.3.mingw.1
  attr: fix segfault in gitattributes parsing code


* The 'master' branch has these since the last announcement
  in addition to the above.

Benoit Sigoure (5):
  git-svn: add a generic tree traversal to fetch SVN properties
  git-svn: implement git svn create-ignore
  git-svn: add git svn propget
  git-svn: add git svn proplist
  git-svn: simplify the handling of fatal errors

Chris Pettitt (1):
  git-p4 support for perforce renames.

Eygene Ryabinkin (1):
  git-svn: use "no warnings 'once'" to disable false-positives

Jeff King (3):
  git-gc: improve wording of --auto notification
  Documentation/git-gc: explain --auto in description
  Documentation/git-gc: improve description of --auto

Johannes Schindelin (1):
  Deduce exec_path also from calls to git with a relative path

Johannes Sixt (1):
  gitk: Do not pick up file names of "copy from" lines

Jonathan del Strother (1):
  gitk: Add support for OS X mouse wheel

Junio C Hamano (2):
  git-am: make the output quieter.
  git-am: fix typo in the previous one.

Linus Torvalds (1):
  optimize diffcore-delta by sorting hash entries.

Luke Lu (1):
  gitweb: speed up project listing on large work trees by limiting
      find depth

Marius Storm-Olsen (1):
  Teach core.autocrlf to 'git blame'

Michael Witten (1):
  git-cvsexportcommit.perl: git-apply no longer needs --binary

Paul Mackerras (3):
  gitk: Check that we are running on at least Tcl/Tk 8.4
  gitk: Avoid an error when cherry-picking if HEAD has moved on
  gitk: Fix "can't unset prevlines(...)" Tcl error

Ralf Wildenhues (1):
  git-cherry-pick: improve description of -x.

René Scharfe (1):
  Correct some sizeof(size_t) != sizeof(unsigned long) typing errors

Sam Vilain (1):
  gitk: disable colours when calling git log

Shawn O. Pearce (2):
  Improved const correctness for strings
  Use PRIuMAX instead of 'unsigned long long' in show-index

Simon Hausmann (1):
  git-p4: When skipping a patch as part of "git-p4 submit" make sure
      we correctly revert to the previous state of the files using
      "p4 revert".

koreth@midwinter.com (1):
  Add a message explaining that automatic GC is about to start

^ permalink raw reply

* Re: [PATCH] "git help" and "git help -a" shouldn't exit(1) unless they error
From: Scott Parish @ 2007-10-22  6:19 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20071022054741.GP14735@spearce.org>

On Mon, Oct 22, 2007 at 01:47:41AM -0400, Shawn O. Pearce wrote:

> The issue here is t0000-basic.sh runs "../git" and tests that the
> exit status is 1.  If it isn't (the patch above makes it 0) we just
> abort the test suite entirely.

Shoot, i hadn't realized i had effected the "git" case. I'll
look into this further.

By the way, should i expect all the tests to pass from the unmodified
public HEAD? (they don't for me)

sRp

-- 
Scott Parish
http://srparish.net/

^ permalink raw reply

* Re: [PATCH] "git help -a" should search all exec_paths and PATH
From: Scott Parish @ 2007-10-22  6:32 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20071022053016.GN14735@spearce.org>

On Mon, Oct 22, 2007 at 01:30:17AM -0400, Shawn O. Pearce wrote:

> fchdir() isn't as portable as Git currently is.  Thus far we have
> avoided using fchdir().  Requiring it here for something as "simple"
> as listing help is not a good improvement as it will limit who can
> run git-help.  Why can't you stat the individual entries by joining
> the paths together?

I hadn't realized it wasn't portable, but i do see that there's no POSIX
entry in its man page. I was actually looking to use getcwd, but its
man page had suggested using this open()/fchdir() method.

Anyway, is there a reason to avoid changing the directory? If not
i'm tempted to take the approach that j.sixt suggested--not restoring
the cwd since we're exiting anyway. I don't have any good reason
to not do the string manipulation, but why do something more
complicated then necessary?

sRp

-- 
Scott Parish
http://srparish.net/

^ permalink raw reply

* What's cooking in git/spearce.git (topics)
From: Shawn O. Pearce @ 2007-10-22  6:32 UTC (permalink / raw)
  To: git

Here are the topics that have been cooking.  Commits prefixed
with '-' are only in 'pu' while commits prefixed with '+' are
in 'next'.  The topics list the commits in reverse chronological
order.

* cc/skip (Mon Oct 22 07:49:39 2007 +0200) 9 commits
 - Bisect: add a "bisect replay" test case.
 - Bisect: add "bisect skip" to the documentation.
 - Bisect: factorise "bisect_{bad,good,skip}" into "bisect_state".
 - Bisect: factorise some logging into "bisect_write".
 - Bisect: factorise "bisect_write_*" functions.
 - Bisect: implement "bisect skip" to mark untestable revisions.
 - Bisect: fix some white spaces and empty lines breakages.
 - rev-list documentation: add "--bisect-all".
 - rev-list: implement --bisect-all

Recently updated to use "skip".  I haven't had a chance to look at
this series so it just parked in pu for now.

* ds/gitweb (Mon Oct 22 10:28:03 2007 +1000) 1 commit
 + gitweb: Provide title attributes for abbreviated author names.

* lt/rename (Sun Oct 21 16:59:03 2007 -0700) 2 commits
 - Linear-time/space rename logic (exact renames only)
 - Split out "exact content match" phase of rename detection

t4001-diff-rename.sh failed while running tests on pu.  I'm pretty
sure its this topic from Linus.  Need to look at it further.

* js/PATH (Sun Oct 21 22:59:01 2007 +0100) 1 commit
 - execv_git_cmd(): also try PATH if everything else fails.

I raised a concern about GIT_EXEC_PATH="" making $PATH search before
the compiled in path, which is certainly new behavior and I don't
think its quite what was intended.  Parked in pu until I hear back.

* sp/help-exit0 (Sun Oct 21 14:47:45 2007 -0700) 1 commit
 . "git help" and "git help -a" shouldn't exit(1) unless they error

Breaks things because "git" (no arguments) no exits successful,
which is less than ideal.  Only "git help" and "git help branch"
should be exiting successful.

* ja/shorthelp (Sun Oct 21 01:41:41 2007 +0300) 1 commit
 + On error, do not list all commands, but point to --help option

This I like.  We'll see what the list thinks while using next.  :)

* db/fetch-pack (Sat Oct 20 16:03:49 2007 -0400) 60 commits
 + Define compat version of mkdtemp for systems lacking it
 + Avoid scary errors about tagged trees/blobs during git-fetch
 + fetch: if not fetching from default remote, ignore default merge
 + Support 'push --dry-run' for http transport
 + Support 'push --dry-run' for rsync transport
 + Fix 'push --all branch...' error handling
 + Fix compilation when NO_CURL is defined
 + Added a test for fetching remote tags when there is not tags.
 + Fix a crash in ls-remote when refspec expands into nothing
 ... and many many more ...

I think this is just about ready to graduate to master.  I haven't
seen any major problems with it since the recent fixes were put in.
I'd like to see it move over soon as a number of other topics
are based upon the tip of this topic rather than master itself.
But obviously code quality is more important than topic organization.

* js/forkexec (Fri Oct 19 21:48:06 2007 +0200) 74 commits
 + Use the asyncronous function infrastructure to run the content
   filter.
 + Avoid a dup2(2) in apply_filter() - start_command() can do it for
   us.
 + t0021-conversion.sh: Test that the clean filter really cleans
   content.
 + upload-pack: Run rev-list in an asynchronous function.
 + upload-pack: Move the revision walker into a separate function.
 + Use the asyncronous function infrastructure in builtin-fetch-
   pack.c.
 + Add infrastructure to run a function asynchronously.
 + upload-pack: Use start_command() to run pack-objects in
   create_pack_file().
 + Have start_command() create a pipe to read the stderr of the
   child.
 + Use start_comand() in builtin-fetch-pack.c instead of explicit
   fork/exec.
 + Use run_command() to spawn external diff programs instead of
   fork/exec.
 + Use start_command() to run content filters instead of explicit
   fork/exec.
 + Use start_command() in git_connect() instead of explicit
   fork/exec.
 + Change git_connect() to return a struct child_process instead of a
   pid_t.
 ... db/fetch-pack begins here ...

This looked sane to me and makes it easier for the MinGW port.
Plus its an overal reduction in code, reusing the run-command
framework to avoid lots of ugly pipe() and dup2() calls.  Its
parked in next for a while to get some testing but is probably
fine to move to master in the near future.

* tf/afs (Fri Oct 19 07:38:16 2007 -0500) 1 commit
 - Better support AFS hardlinking across object directories

I'd rather rewrite this by putting the temporary files directly into
their final object directory, then hardlinking within that directory.
This should work on Coda and AFS, leaving only the FAT filesystem
as the odd-duck that needs to use rename().  But maybe I'm just
being really paranoid about not allowing an object to be replaced.

* jk/terse-fetch (Fri Oct 19 03:40:57 2007 -0400) 62 commits
 - park
 - git-fetch: mega-terse fetch output
 ... db/fetch-pack begins here ...

Much discussion on the list about this topic.  I think we may have
come to an agreement about what the output should look like, but
this topic doesn't implement that output format.  Someone needs to
either update this topic or rewrite it.  Starting from Jeff King's
patch makes things a lot easier.

* np/progress (Fri Oct 19 01:01:40 2007 -0400) 9 commits
 + Stop displaying "Pack pack-$ID created." during git-gc
 + Teach prune-packed to use the standard progress meter
 + Change 'Deltifying objects' to 'Compressing objects'
 + fix for more minor memory leaks
 + fix const issues with some functions
 + pack-objects.c: fix some global variable abuse and memory leaks
 + pack-objects: no delta possible with only one object in the list
 + cope with multiple line breaks within sideband progress messages
 + more compact progress display

I really like the new progress display that Nico implemented here,
and also the terser and more user friendly output from git-repack.
Needs more time for testing, but its pretty obvious code changes
and should be able to graduate to master in the near future.

* sp/mergetool (Thu Oct 18 12:25:34 2007 +0200) 3 commits
 + mergetool: avoid misleading message "Resetting to default..."
 + mergetool: add support for ECMerge
 + mergetool: use path to mergetool in config var
   mergetool.<tool>.path

Probably fine for master as-is.  I personally don't use mergetool so
I'd appreciate an Ack from someone who does that these are working
well for them.

* jk/send-pack (Thu Oct 18 02:17:46 2007 -0400) 2 commits
 + t5516: test update of local refs on push
 + send-pack: don't update tracking refs on error

This probably should graduate to master soon.  It just delays
updating the tracking refs until after we've made sure all refs
were updated.  I'll give it a few more days and then likely kick
it over to master.

* js/rebase (Wed Oct 17 10:31:35 2007 +0100) 1 commit
 + Fixing path quoting in git-rebase

Simple change, but rebase is a key tool.  I'll probably give this
a few more days and then kick it over.

* js/reflog-delete (Wed Oct 17 02:50:45 2007 +0100) 1 commit
 + Teach "git reflog" a subcommand to delete single entries

Waiting to hear if we're doing anything further with this.  it was
originally created to help "git stash" perform a pop, but nobody
has come forth with a patch for git-stash that uses this feature.
I'd like to have a real use for it that actually tests this code
in a more production setting before it merges to master.

* dz/color-addi (Tue Oct 16 21:42:23 2007 -0400) 1 commit
 - Add color support to git-add--interactive

I'm just parking this here in case anyone wants to pick this up and
continue it further.  I described on list to the original author
a number of problems with why this isn't in next yet; the author
said they will need a little bit of time before they can address
that list.

* ph/parseopt (Mon Oct 15 23:06:02 2007 +0200) 22 commits
 - Make builtin-pack-refs.c use parse_options.
 - Make builtin-name-rev.c use parse_options.
 - Make builtin-count-objects.c use parse_options.
 - Make builtin-fsck.c use parse_options.
 - Update manpages to reflect new short and long option aliases
 - Make builtin-for-each-ref.c use parse-opts.
 - Make builtin-symbolic-ref.c use parse_options.
 - Make builtin-update-ref.c use parse_options
 - Make builtin-revert.c use parse_options.
 - Make builtin-describe.c use parse_options
 - Make builtin-branch.c use parse_options.
 - Make builtin-mv.c use parse-options
 - Make builtin-rm.c use parse_options.
 - Port builtin-add.c to use the new option parser.
 - parse-options: allow callbacks to take no arguments at all.
 - parse-options: Allow abbreviated options when unambiguous
 - Add shortcuts for very often used options.
 - parse-options: make some arguments optional, add callbacks.
 - Rework make_usage to print the usage message immediately
 - Add tests for parse-options.c
 - parse-options: be able to generate usages automatically
 - Add a simple option parser.

There's actually a few other commits (3?) missing from the above
list that are safely parked away in my tree.  I'm most of the way
through reviewing these and have made a few bug fixes and style
nit corrections in the earlier parts of the series.  I'm going to
try and finish working through this series tomorrow and then will
probably merge it to next.

The other 3 commits are hanging off to the side as 2 of them are
for the top of db/fetch-pack (to port builtin-fetch and friends
to the new option parser) and the 3rd is a somewhat questionable
change due to needing to rename a "-h" to a "-H".  I just got
tired of rebasing these 3 other commits onto the respective topics.
I'm waiting for the core option parser (above) to freeze on a commit
SHA-1 before I deal with these again.

* sp/push-refspec (Sun Oct 14 10:54:45 2007 +0200) 6 commits
 - push, send-pack: use same rules as git-rev-parse to resolve
   refspecs
 - add ref_cmp_full_short() comparing full ref name with a short name
 - push, send-pack: support pushing HEAD to real ref name
 - rev-parse: teach "git rev-parse --symbolic" to print the full ref
   name
 - add get_sha1_with_real_ref() returning full name of ref on demand
 - push, send-pack: fix test if remote branch exists for colon-less
   refspec

I've briefly looked at this series and there's reasons why its not
in next yet.  Its actually something that I'm interested in seeing
fixed as the current behavior of how git-push matches refs on the
remote side is just plain nuts.  I'll look at it further after I
get ph/parseopt and cc/skip into next.

* jc/spht (Tue Oct 2 18:00:27 2007 -0700) 1 commit
 - git-diff: complain about >=8 consecutive spaces in initial indent
* jc/nu (Mon Oct 1 19:35:12 2007 -0700) 2 commits
 - PARK a bit more work
 - (PARK) WIP

I inherited these from Junio.  No change.

* kh/commit (Mon Sep 17 20:06:47 2007 -0400) 4 commits
 + Export rerere() and launch_editor().
 + Introduce entry point add_interactive and add_files_to_cache
 + Enable wt-status to run against non-standard index file.
 + Enable wt-status output to a given FILE pointer.

Waiting on ph/parseopt (above) and other stuff for builtin-commit.

* jc/pathspec (Thu Sep 13 13:38:19 2007 -0700) 3 commits
 - pathspec_can_match(): move it from builtin-ls-tree.c to tree.c
 - ls-tree.c: refactor show_recursive() and rename it.
 - tree-diff.c: split out a function to match a single pattern.

Also inherited from Junio.  No change.

* js/stash-create (Mon Jul 9 00:51:23 2007 -0700) 2 commits
 + rebase: allow starting from a dirty tree.
 + stash: implement "stash create"

I actually had the "dirty work tree stash in rebase" thing trip on
me recently and I got a little pissed off about it.  The behavior
was not what I expected, nor what I wanted, at that particular point
in time.  Actually I wanted git-rebase to stop and *not* attempt
the rebase because of the dirty work tree.  So I'm not feeling the
"auto stash" love right now.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] "git help" and "git help -a" shouldn't exit(1) unless they error
From: Shawn O. Pearce @ 2007-10-22  6:37 UTC (permalink / raw)
  To: Scott Parish; +Cc: git
In-Reply-To: <20071022061918.GM16291@srparish.net>

Scott Parish <sRp@srparish.net> wrote:
> On Mon, Oct 22, 2007 at 01:47:41AM -0400, Shawn O. Pearce wrote:
> 
> > The issue here is t0000-basic.sh runs "../git" and tests that the
> > exit status is 1.  If it isn't (the patch above makes it 0) we just
> > abort the test suite entirely.
> 
> Shoot, i hadn't realized i had effected the "git" case. I'll
> look into this further.
> 
> By the way, should i expect all the tests to pass from the unmodified
> public HEAD? (they don't for me)

Yes.  I only push maint, master and next if they pass all tests.
If something doesn't pass I rewind the branch until it does (of
course I only rewind back to what I've previously published).

I do however push a broken pu.  Because individual topics in there
may be valid, but one or two may also be broken.  The pu branch
is meant to be a place to obtain a specific topic of interest
from so you can work further on it, or test it.  Like tonight.
pu compiles but doesn't pass the tests.

So if you are seeing one or more tests fail please go run that
specific test(s) with "-i -v" and either come up with a fix for
the test, or at least post the output of that to the mailing
list so someone else can have a chance to resolve the problem.

Since everything passes here for me on Linux/x86_64 I'm guessing
its a platform specific issue.  We still should fix it.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] "git help -a" should search all exec_paths and PATH
From: Shawn O. Pearce @ 2007-10-22  6:39 UTC (permalink / raw)
  To: Scott Parish; +Cc: git
In-Reply-To: <20071022063201.GN16291@srparish.net>

Scott Parish <sRp@srparish.net> wrote:
> On Mon, Oct 22, 2007 at 01:30:17AM -0400, Shawn O. Pearce wrote:
> 
> > fchdir() isn't as portable as Git currently is.
> 
> I hadn't realized it wasn't portable, but i do see that there's no POSIX
> entry in its man page. I was actually looking to use getcwd, but its
> man page had suggested using this open()/fchdir() method.
> 
> Anyway, is there a reason to avoid changing the directory? If not
> i'm tempted to take the approach that j.sixt suggested--not restoring
> the cwd since we're exiting anyway. I don't have any good reason
> to not do the string manipulation, but why do something more
> complicated then necessary?

Yea, that was another thought I had.  You probably can just chdir(),
list, exit, and not worry about going back to the previous directory.
And more complicated is always a bad idea.  Keep it simple, 'cause
us gits like it that way.  :-)

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Be nice with compilers that do not support runtime paths at all.
From: Shawn O. Pearce @ 2007-10-22  6:44 UTC (permalink / raw)
  To: Benoit SIGOURE; +Cc: git list
In-Reply-To: <09169ECD-19E1-44D1-8539-71EBBA3826A8@lrde.epita.fr>

Benoit SIGOURE <tsuna@lrde.epita.fr> wrote:
> >On Oct 4, 2007, at 1:18 AM, Junio C Hamano wrote:
> >>Benoit Sigoure <tsuna@lrde.epita.fr> writes:
> >>
> >>If we do not care about supporting too old GNU make, we can do
> >>this by first adding this near the top:
> >>
> >>        ifndef NO_RPATH
> >>        LINKER_PATH = -L$(1) $(CC_LD_DYNPATH)$(1)
> >>        else
> >>        LINKER_PATH = -L$(1)
> >>        endif
> >>
> >>and then doing something like:
> >>
> >>	CURL_LIBCURL = $(call LINKER_PATH,$(CURLDIR)/$(lib))
> >>	OPENSSL_LINK = $(call LINKER_PATH,$(OPENSSLDIR)/$(lib))
> >>
> >>to make it easier to read and less error prone.
> >
> >Yes.  I can rework the patch, but the question is: do you care  
> >about old GNU make?  Can I rewrite the patch with this feature?
> 
> I know Junio is still offline but maybe someone else has an objection  
> against this?

How old of a GNU make are talking about here?  The above is certainly
a lot nicer to read, but I'd hate to suddenly ship a new Git that
someone cannot compile because their GNU make is too old.

GNU make is fortunately pretty easy to compile, so it shouldn't be
that difficult for someone to build a newer version if they had to,
but why make them go through all that extra work just to install
a new Git?

What about using a small helper shell script and using $(shell)
instead of $(call)?

So I guess in short I think I was in agreement with Junio a while
ago on this, which was that I don't want to require a newer GNU
make than we already require our users to have.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH, take 1] Linear-time/space rename logic (exact renames only)
From: Jeff King @ 2007-10-22  6:47 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Git Mailing List, Junio C Hamano, Shawn O. Pearce, David Kastrup
In-Reply-To: <alpine.LFD.0.999.0710211603200.10525@woody.linux-foundation.org>

On Sun, Oct 21, 2007 at 04:59:03PM -0700, Linus Torvalds wrote:

> What it does is to rather than iterate over all sources and destinations 
> and checking if they are identical (which is O(src*dst)), it hashes each 
> of the sources and destinations into a hash table, using the SHA1 hash of 
> the contents as the hash. That's O(n+m). It then walks the hash table 
> (which is also O(m+n) in size), and only pairs up files for comparison 
> that hashed to the same spot.
> 
> Doing this for more than just the exact same contents would be basically 
> the same thing, except it starts hashing up fingerprints of the contents 
> and linking up file pairs that get linked up by those fingerprints. More 
> involved, but not impossible.

Hrm. For the inexact case, it seems like you should be able to do
something like this:

 1. put the content fingerprint hashes in a hash table
 2. create a single src * dst table of scores
 3. walk the hash table; for each bucket in which there are collisions,
    find every pair in that bucket, and add to the similarity score in
    the big score table
 4. for each src in the similarity table, find the maximum dest

Step 1 is clearly O(n+m). Step 2 allocates O(n*m) memory, but we only
need a single integer in each slot. The outer walk in step 3 is
O(fingerprints), which is, O(content_size * (n+m)). The inner loop can
actually be worst case O(n*m), but is more likely to be a handful of
pairs (assuming that for any fingerprint chunk, it is only going to
exist in a few files; you would get worst case if you were comparing a
bunch of files with identical contents). And step 4 is actually going to
end up being O(n*m), since you have to find the best match for each.

So there is actually still O(n*m) behavior, but I wonder if we are
tightening the O(n*m) loop enough that we will see improvement.

Hrm. I wonder if we could keep a "best" pointer for each src file, and
when we update the score for a given src/dest pair, check for a new
best. That would add more to step 3, but make step 4 O(n).

And of course the n*m memory is going to bottleneck. I think for 1000 *
1000, it should be fine, but if you want to try 100,000 * 100,000, you
are going to need tens of gigabytes. And it's going to be very sparse.
So perhaps a hash table with keys of src+dst mapped to scores. And then
we could sort the whole table afterwards, and just run through it
linearly, which helps step 4.

>  - it looks ok, and I've tested it some, but this needs more people 
>    looking at it.

Overall it looks like a sane approach. My comments are below.

>  - in fact, the big optimization isn't the actual hash table, but the 
>    independent and much simpler "diff_filespec->used" optimization for a 
>    deleted filename that was used for a rename/copy.

Do you have separate timings? The "diff_filespec->used" optimization
appears to be cutting out an O(n*m) strcmp. If it is most of the
optimization, I wonder if the complexity of the hash change is worth it
(although I find the new code easier to read, so maybe it is worth it on
those grounds alone).

> +struct file_similarity {
> +	int src_dst, index;

It took me a while to figure out all of the meanings of src_dst; maybe a
comment is in order?

> +static int find_identical_files(struct file_similarity *src,
> +				struct file_similarity *dst)

Your function naming is a bit confusing. You have find_identical_files,
find_same_files, and find_exact_renames, all of which do the same thing,
but for different levels of input. Perhaps the names should reflect how
they are different?

> +static int find_identical_files(struct file_similarity *src,
> +				struct file_similarity *dst)
> +{
> +	int renames = 0;
> +	do {
> +		struct diff_filespec *one = src->filespec;
> +		struct file_similarity *p, *best;
> +		int i = 100;
> +
> +		best = NULL;
> +		for (p = dst; p; p = p->next) {
> +			struct diff_filespec *two = p->filespec;
> +
> +			/* Already picked as a destination? */
> +			if (!p->src_dst)
> +				continue;
> +			/* False hash collission? */
> +			if (hashcmp(one->sha1, two->sha1))
> +				continue;
> +			best = p;
> +			if (basename_same(one, two))
> +				break;
> +
> +			/* Too many identical alternatives? Pick one */
> +			if (!--i)
> +				break;
> +		}
> +		if (best) {
> +			best->src_dst = 0;
> +			record_rename_pair(best->index, src->index, MAX_SCORE);
> +			renames++;
> +		}
> +	} while ((src = src->next) != NULL);
> +	return renames;
> +}

The "too many identical alternatives" sanity check is interesting. It
can produce suboptimal results if, e.g., the 101st entry has the same
basename. I suppose it is necessary to prevent a worst case "oops, all
of these files are identical" O(n*m) behavior. But generally I would
favor "always correct, pathological cases slow" to "always fast,
pathological cases incorrect". But maybe the basename heuristic isn't
worth considering "correct" in this case (and honestly, I doubt anyone
will hit the 100 limit anyway).

> +/*
> + * Note: the rest of the rename logic depends on this
> + * phase also populating all the filespecs for any
> + * entry that isn't matched up with an exact rename.
> + */
> +static int free_file_table(void *ptr)
> +{
> +	struct file_similarity *p = ptr;
> +	do {
> +		struct file_similarity *entry = p;
> +		p = p->next;
> +
> +		/* Stupid special case, see note above! */
> +		diff_populate_filespec(entry->filespec, 0);
> +		free(entry);
> +	} while (p);
> +	return 0;
> +}

diff_populate_filespec knows whether or not it needs to do any work. I
wonder if those parts of the rename process that need it should just
call it unconditionally. It seems like a fragile dependency. Besides
which...

> +static unsigned int hash_filespec(struct diff_filespec *filespec)
> +{
> +	unsigned int hash;
> +	if (!filespec->sha1_valid) {
> +		if (diff_populate_filespec(filespec, 0))
> +			return 0;
> +		hash_sha1_file(filespec->data, filespec->size, "blob", filespec->sha1);
> +	}
> +	memcpy(&hash, filespec->sha1, sizeof(hash));
> +	return hash;
> +}

...if everything has been through this hash function, why do we need to
populate again upon freeing?

> +static struct hash_table_entry *lookup_hash_entry(unsigned int hash, struct hash_table *table)
> +{
> +	unsigned int size = table->size, nr = hash % size;
> +	struct hash_table_entry *array = table->array;
> +
> +	while (array[nr].ptr) {
> +		if (array[nr].hash == hash)
> +			break;
> +		nr++;
> +		if (nr >= size)
> +			nr = 0;
> +	}
> +	return array + nr;
> +}

Rather than having buckets where collisions extend in a list within the
bucket, it looks like you are just overflowing into the next bucket.
It seems like you could end up with pretty bad "runs" of full buckets.
E.g., bucket 1 has a collision, so it bleeds into bucket 2.  Now when we
place something in bucket 2, it has to skip past bucket 1's overflow.
And a collision in bucket 2 means we have to skip the overflow for
buckets 1 _and_ 2. And so on, until we can resync by finding enough free
buckets to accept all of our collisions. So it depends on keeping a lot
of holes in the hash structure, which I see that you do, but I wonder
what the optimal value is.

I assume you chose this method to reduce memory fragmentation, which
makes sense. And maybe there is a simple answer that "50-75% free gives
good results over evenly distributed hashes." Though perhaps we should
also consider clustered hashes (especially if we want to put the
fingerprint hashes directly in here).

> +/*
> + * Insert a new hash entry pointer into the table.
> + *
> + * If that hash entry already existed, return the pointer to
> + * the existing entry (and the caller can create a list of the
> + * pointers or do anything else). If it didn't exist, return
> + * NULL (and the caller knows the pointer has been inserted).
> + */
> +static void **insert_hash_entry(unsigned int hash, void *ptr, struct hash_table *table)

This calling convention seems a bit clunky. Since all callers have to
check for hash collision anyway, why not just unconditionally return the
pointer to the data ptr (which will itself be NULL if this is the first
entry)? Then you can drop the conditional, and:

  pos = insert_hash(hash, entry, table);
  if (pos) {
    entry->next = *pos;
    *pos = entry;
  }

can simply become:

  pos = insert_hash(hash, table);
  entry->next = *pos;
  *pos = entry;

> +void **insert_hash(unsigned int hash, void *ptr, struct hash_table *table)
> +{
> +	unsigned int nr = table->nr;
> +	if (nr >= table->size/2)
> +		grow_hash_table(table);
> +	return insert_hash_entry(hash, ptr, table);
> +}

Style nitpick: the local "nr" is rather pointless.

-Peff

^ permalink raw reply

* Re: [PATCH] Be nice with compilers that do not support runtime paths at  all.
From: Brian Dessent @ 2007-10-22  6:52 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Benoit SIGOURE, git list
In-Reply-To: <20071022064454.GV14735@spearce.org>

"Shawn O. Pearce" wrote:

> How old of a GNU make are talking about here?  The above is certainly

According to the NEWS file, $(call) was added to GNU make v3.78,
released 1999-09-22.

Brian

^ permalink raw reply

* Re: What's cooking in git/spearce.git (topics)
From: Jeff King @ 2007-10-22  6:59 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20071022063222.GS14735@spearce.org>

On Mon, Oct 22, 2007 at 02:32:22AM -0400, Shawn O. Pearce wrote:

> * jk/terse-fetch (Fri Oct 19 03:40:57 2007 -0400) 62 commits
>  - park
>  - git-fetch: mega-terse fetch output
>  ... db/fetch-pack begins here ...
> 
> Much discussion on the list about this topic.  I think we may have
> come to an agreement about what the output should look like, but
> this topic doesn't implement that output format.  Someone needs to
> either update this topic or rewrite it.  Starting from Jeff King's
> patch makes things a lot easier.

I will eventually get around to rewriting this (it seems the list
comments have died down), but it is much more interesting to play with
Linus' rename stuff tonight. :) If somebody else wants to take a stab,
please go ahead.

-Peff

^ permalink raw reply

* Re: [PATCH, take 1] Linear-time/space rename logic (exact renames only)
From: Sven Verdoolaege @ 2007-10-22  7:07 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Git Mailing List, Junio C Hamano, Shawn O. Pearce, David Kastrup,
	Jeff King
In-Reply-To: <alpine.LFD.0.999.0710211603200.10525@woody.linux-foundation.org>

On Sun, Oct 21, 2007 at 04:59:03PM -0700, Linus Torvalds wrote:
> +static int find_same_files(void *ptr)
> +{
> +	struct file_similarity *p = ptr;
> +	struct file_similarity *src = NULL, *dst = NULL;
> +
> +	/* Split the hash list up into sources and destinations */
> +	do {
> +		struct file_similarity *entry = p;
> +		p = p->next;
> +		if (entry->src_dst < 0) {
> +			entry->next = src;
> +			src = entry;
> +		} else {
> +			entry->next = dst;
> +			dst = entry;
> +		}
> +	} while (p);

Aren't you truncating the ptr list after the first entry here?
(While you still need the whole list in free_file_table.)

skimo

^ permalink raw reply

* Re: What's cooking in git/spearce.git (topics)
From: Jeff King @ 2007-10-22  7:16 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Linus Torvalds, git
In-Reply-To: <20071022063222.GS14735@spearce.org>

On Mon, Oct 22, 2007 at 02:32:22AM -0400, Shawn O. Pearce wrote:

> * lt/rename (Sun Oct 21 16:59:03 2007 -0700) 2 commits
>  - Linear-time/space rename logic (exact renames only)
>  - Split out "exact content match" phase of rename detection
> 
> t4001-diff-rename.sh failed while running tests on pu.  I'm pretty
> sure its this topic from Linus.  Need to look at it further.

Hrm, the problem is that it's not favoring basenames anymore. But I
think it is because the loop in find_identical_files is inside out. For
every source file, it picks the best destination file. But I think we
want to do the opposite: for every destination file, pick the best
source file. Otherwise, if a non-basename source file is connected with
a particular destination file, we no longer try that destination file
again.

Patch is below (please just squash with the one from Linus).

diff --git a/diffcore-rename.c b/diffcore-rename.c
index 05d39db..8881818 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -252,17 +252,18 @@ static int find_identical_files(struct file_similarity *src,
 {
 	int renames = 0;
 	do {
-		struct diff_filespec *one = src->filespec;
+		struct diff_filespec *one = dst->filespec;
 		struct file_similarity *p, *best;
 		int i = 100;
 
 		best = NULL;
-		for (p = dst; p; p = p->next) {
+		for (p = src; p; p = p->next) {
 			struct diff_filespec *two = p->filespec;
 
-			/* Already picked as a destination? */
+			/* Already picked as a source? */
 			if (!p->src_dst)
 				continue;
+
 			/* False hash collission? */
 			if (hashcmp(one->sha1, two->sha1))
 				continue;
@@ -276,10 +277,10 @@ static int find_identical_files(struct file_similarity *src,
 		}
 		if (best) {
 			best->src_dst = 0;
-			record_rename_pair(best->index, src->index, MAX_SCORE);
+			record_rename_pair(dst->index, best->index, MAX_SCORE);
 			renames++;
 		}
-	} while ((src = src->next) != NULL);
+	} while ((dst = dst->next) != NULL);
 	return renames;
 }
 

^ permalink raw reply related

* Re: [PATCH, take 1] Linear-time/space rename logic (exact renames only)
From: Jeff King @ 2007-10-22  7:21 UTC (permalink / raw)
  To: skimo
  Cc: Linus Torvalds, Git Mailing List, Junio C Hamano, Shawn O. Pearce,
	David Kastrup
In-Reply-To: <20071022070750.GM1179MdfPADPa@greensroom.kotnet.org>

On Mon, Oct 22, 2007 at 09:07:50AM +0200, Sven Verdoolaege wrote:

> Aren't you truncating the ptr list after the first entry here?
> (While you still need the whole list in free_file_table.)

Yes, good eyes. And because we actually reverse the list, it's not as
simple as just sticking the two broken up pieces together again; the
original head must end up as the head of the list after they are glued
together again, but it is actually the tail of one of the lists.

-Peff

^ permalink raw reply

* Re: What's cooking in git/spearce.git (topics)
From: Pierre Habouzit @ 2007-10-22  7:24 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20071022063222.GS14735@spearce.org>

[-- Attachment #1: Type: text/plain, Size: 2790 bytes --]

On Mon, Oct 22, 2007 at 06:32:22AM +0000, Shawn O. Pearce wrote:
> * ph/parseopt (Mon Oct 15 23:06:02 2007 +0200) 22 commits
>  - Make builtin-pack-refs.c use parse_options.
>  - Make builtin-name-rev.c use parse_options.
>  - Make builtin-count-objects.c use parse_options.
>  - Make builtin-fsck.c use parse_options.
>  - Update manpages to reflect new short and long option aliases
>  - Make builtin-for-each-ref.c use parse-opts.
>  - Make builtin-symbolic-ref.c use parse_options.
>  - Make builtin-update-ref.c use parse_options
>  - Make builtin-revert.c use parse_options.
>  - Make builtin-describe.c use parse_options
>  - Make builtin-branch.c use parse_options.
>  - Make builtin-mv.c use parse-options
>  - Make builtin-rm.c use parse_options.
>  - Port builtin-add.c to use the new option parser.
>  - parse-options: allow callbacks to take no arguments at all.
>  - parse-options: Allow abbreviated options when unambiguous
>  - Add shortcuts for very often used options.
>  - parse-options: make some arguments optional, add callbacks.
>  - Rework make_usage to print the usage message immediately
>  - Add tests for parse-options.c
>  - parse-options: be able to generate usages automatically
>  - Add a simple option parser.
>
> There's actually a few other commits (3?) missing from the above
> list that are safely parked away in my tree.  I'm most of the way
> through reviewing these and have made a few bug fixes and style
> nit corrections in the earlier parts of the series.  I'm going to
> try and finish working through this series tomorrow and then will
> probably merge it to next.


> The other 3 commits are hanging off to the side as 2 of them are
> for the top of db/fetch-pack (to port builtin-fetch and friends
> to the new option parser) and the 3rd is a somewhat questionable
> change due to needing to rename a "-h" to a "-H".  I just got
> tired of rebasing these 3 other commits onto the respective topics.
> I'm waiting for the core option parser (above) to freeze on a commit
> SHA-1 before I deal with these again.

  You also can ask me to resubmit them when the first round of parseopt
is merged. I'm actually waiting for that before I work on it again
(that's not the sole reason, $work ate my time as well, so I'm not
pressuring ;P) and I can send those again when things are more ready for
them.

  Like I said to you on IRC, I intend to hack a way to ask parse-options
to dump its options in a machine parseable way, to help {z,ba}sh
completions authors. And obviously I intend to migrate even more
builtins :)

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH] git-format-patch: Don't number patches when there's only one
From: Andreas Ericsson @ 2007-10-21  8:13 UTC (permalink / raw)
  To: git; +Cc: spearce

[PATCH 1/1] looks a bit silly, and automagically handling
this in git-format-patch makes some scripting around it a
lot more pleasant.

Signed-off-by: Andreas Ericsson <ae@op5.se>
---
 Documentation/git-format-patch.txt |    3 ++-
 builtin-log.c                      |    4 ++++
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index c9857a2..9f16951 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -56,7 +56,8 @@ If -o is specified, output files are created in <dir>.  Otherwise
 they are created in the current working directory.
 
 If -n is specified, instead of "[PATCH] Subject", the first line
-is formatted as "[PATCH n/m] Subject".
+is formatted as "[PATCH n/m] Subject" if the revision range to
+format contains more than one commit.
 
 If given --thread, git-format-patch will generate In-Reply-To and
 References headers to make the second and subsequent patch mails appear
diff --git a/builtin-log.c b/builtin-log.c
index e8b982d..5c48f4d 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -642,6 +642,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		list[nr - 1] = commit;
 	}
 	total = nr;
+
+	/* don't number patches when there's only one */
+	if (total == 1)
+		numbered = 0;
 	if (numbered)
 		rev.total = total + start_number - 1;
 	rev.add_signoff = add_signoff;
-- 
1.5.3.4.1273.g725c19

^ permalink raw reply related

* [PATCH] Let users override name of per-directory ignore file
From: Andreas Ericsson @ 2007-10-15 12:09 UTC (permalink / raw)
  To: git; +Cc: spearce

When collaborating with projects managed by some other
scm, it often makes sense to have git read that other
scm's ignore-files. This patch lets git do just that, if
the user only tells it the name of the per-directory
ignore file by specifying the newly introduced git config
option 'core.ignorefile'.

Theoretically, this could cause problems when projects get
ported from some other scm to git, but in practice that
is a moot point, as such changes are always followed by a
flagday anyway.

Signed-off-by: Andreas Ericsson <ae@op5.se>
---
 Documentation/config.txt    |   10 ++++++++++
 Documentation/gitignore.txt |    5 +++++
 builtin-add.c               |   11 +++++++++--
 wt-status.c                 |    9 ++++++++-
 4 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index edf50cd..5170e27 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -275,6 +275,16 @@ You probably do not need to adjust this value.
 +
 Common unit suffixes of 'k', 'm', or 'g' are supported.
 
+core.ignorefile::
+	Tells git to use a different file for per-directory ignores.
+	This is useful when one wishes to use git for a project
+	when the upstream repository is managed by some other SCM
+	whose ignore-file formats are the same as that of git.
+	For example, setting core.ignorefile to .svnignore in
+	repos where one interacts with the upstream project repo
+	using gitlink:git-svn[1] will make a both SVN users and
+	your own repo ignore the same files.
+
 core.excludesfile::
 	In addition to '.gitignore' (per-directory) and
 	'.git/info/exclude', git looks into this file for patterns
diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index e8b8581..f82eac6 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -32,6 +32,11 @@ precedence, the last matching pattern decides the outcome):
    `.gitignore` file.  A project normally includes such
    `.gitignore` files in its repository, containing patterns for
    files generated as part of the project build.
+   The name of the `.gitignore` file can be changed by setting
+   the configuration variable 'core.ignorefile'. This is useful
+   when using git for projects where upstream is using some other
+   SCM. For example, setting 'core.ignorefile' to `.cvsignore`
+   will make git ignore the same files CVS would.
 
  * Patterns read from `$GIT_DIR/info/exclude`.
 
diff --git a/builtin-add.c b/builtin-add.c
index 966e145..c785d99 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -19,6 +19,7 @@ static const char builtin_add_usage[] =
 
 static int take_worktree_changes;
 static const char *excludes_file;
+static const char *ignore_file = ".gitignore";
 
 static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)
 {
@@ -57,7 +58,7 @@ static void fill_directory(struct dir_struct *dir, const char **pathspec,
 	memset(dir, 0, sizeof(*dir));
 	if (!ignored_too) {
 		dir->collect_ignored = 1;
-		dir->exclude_per_dir = ".gitignore";
+		dir->exclude_per_dir = ignore_file;
 		path = git_path("info/exclude");
 		if (!access(path, R_OK))
 			add_excludes_from_file(dir, path);
@@ -144,6 +145,12 @@ static int git_add_config(const char *var, const char *value)
 		excludes_file = xstrdup(value);
 		return 0;
 	}
+	if (!strcmp(var, "core.ignorefile")) {
+		if (!value)
+			die("core.ignorefile without value");
+		ignore_file = xstrdup(value);
+		return 0;
+	}
 
 	return git_default_config(var, value);
 }
@@ -158,7 +165,7 @@ int interactive_add(void)
 static struct lock_file lock_file;
 
 static const char ignore_error[] =
-"The following paths are ignored by one of your .gitignore files:\n";
+"The following paths are ignored:\n";
 
 int cmd_add(int argc, const char **argv, const char *prefix)
 {
diff --git a/wt-status.c b/wt-status.c
index 03b5ec4..103aefe 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -23,6 +23,7 @@ static const char use_add_rm_msg[] =
 static const char use_add_to_include_msg[] =
 "use \"git add <file>...\" to include in what will be committed";
 static const char *excludes_file;
+static const char *ignore_file = ".gitignore";
 
 static int parse_status_slot(const char *var, int offset)
 {
@@ -257,7 +258,7 @@ static void wt_status_print_untracked(struct wt_status *s)
 
 	memset(&dir, 0, sizeof(dir));
 
-	dir.exclude_per_dir = ".gitignore";
+	dir.exclude_per_dir = ignore_file;
 	if (!s->untracked) {
 		dir.show_other_directories = 1;
 		dir.hide_empty_directories = 1;
@@ -370,5 +371,11 @@ int git_status_config(const char *k, const char *v)
 		excludes_file = xstrdup(v);
 		return 0;
 	}
+	if (!strcmp(k, "core.ignorefile")) {
+		if (!v)
+			die("core.ignorefile without value");
+		ignore_file = xstrdup(v);
+		return 0;
+	}
 	return git_default_config(k, v);
 }
-- 
1.5.3.4.1273.g725c19

^ permalink raw reply related

* [PATCH] don't set-group-id on directories on apple
From: Scott R Parish @ 2007-10-22  7:55 UTC (permalink / raw)
  To: git

"git init --shared=all" was failing because chmod was returning
EPERM. According to the man page, the set-group-id behavior is
already default: man 2 mkdir:

  The directory's group ID is set to that of the parent directory
  in which it is created.

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 path.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/path.c b/path.c
index 4260952..4089753 100644
--- a/path.c
+++ b/path.c
@@ -282,8 +282,10 @@ int adjust_shared_perm(const char *path)
 			 : (shared_repository == PERM_EVERYBODY
 			    ? (S_IXGRP|S_IXOTH)
 			    : 0));
+#if !defined(__APPLE__)
 	if (S_ISDIR(mode))
 		mode |= S_ISGID;
+#endif
 	if ((mode & st.st_mode) != mode && chmod(path, mode) < 0)
 		return -2;
 	return 0;
-- 
1.5.3.4.209.g5d1ce-dirty

^ permalink raw reply related

* Re: Git User's Survey 2007 unfinished summary continued
From: Andreas Ericsson @ 2007-10-22  7:59 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Jakub Narebski, Steffen Prohaska, Federico Mena Quintero, git
In-Reply-To: <Pine.LNX.4.64.0710212308540.25221@racer.site>

Johannes Schindelin wrote:
> Hi,
> 
> On Sun, 21 Oct 2007, Andreas Ericsson wrote:
> 
>> Johannes Schindelin wrote:
>>
>>> On Sun, 21 Oct 2007, Jakub Narebski wrote:
>>>
>>>> On 10/20/07, Steffen Prohaska <prohaska@zib.de> wrote:
>>>>
>>>>> Maybe we could group commands into more categories?
>>>>>
>>>>> plumbing: should be hidden from the 'normal' user. Porcelain
>>>>>    should be sufficient for every standard task.
>>>> The problem is division between what is porcelain and what is 
>>>> plumbing. Some commands are right on border (git-fsck, 
>>>> git-update-index, git-rev-parse comes to mind).
>>> Sorry, but my impression from the latest mails was that the commands 
>>> are fine.  What is lacking is a nice, _small_ collection of 
>>> recommended workflows.  And when we have agreed on such a set of 
>>> workflows, we optimize the hell out of them.  Only this time it is not 
>>> performance, but user-friendliness.
>> http://www.kernel.org/pub/software/scm/git/docs/everyday.html would be a 
>> good starting point, I think.
> 
> I don't think so.  Way too few authors were involved in writing this 
> document, so it is not "typical" in and of itself.
> 
> I'd really like people to respond not so much with broad and general 
> statements to my mail (those statements tend to be rather useless to find 
> how to make git more suitable to newbies), but rather with concrete top 
> ten lists of what they do daily.
> 
> My top ten list:
> 
> - git diff
> - git commit
> - git status
> - git fetch
> - git rebase
> - git pull
> - git cherry-pick
> - git bisect
> - git push
> - git add
> 
> So again, I'd like people who did _not_ tweak git to their likings to tell 
> the most common steps they do.  My hope is that we see things that are 
> good practices, but could use an easier user interface.
> 

I'm not so sure we'd want to hide commands that git-gurus simply do not 
use, such as git-blame. In my opinion, we should just locate the highest 
level available of UI tool that implements a particular feature and have 
that listed in the git[- ]<tab> view.

I doubt many people on this list regularly use git-blame but it's a 
command that's definitely non-trivial to script out using only the 
"proper" commands, and CVS/SVN users expect it to be there, so it's 
probably worth listing anyhow.

Similarly, it might be helpful to have help topics the gdb way, like 
"git help patches". It's one of those things that people have come to 
expect from a software tool, so perhaps we should humor them? Given gits 
"every help topic is a man-page" idiom, this shouldn't require any real 
technical effort.

Such topics should probably include
merge/merges/merging - overview of various ways of putting two lines of 
development back together
patch/patches - how to create, send and apply
tags/branches/refs - what they are, why they're good, link to merging

I'm currently swanked with day-job work, but I'll see if I can get some 
prototype docs done later this week and check if it requires any code 
support. If anyone's well-versed in asciidoc HTML-indexing and wants to 
provide pointers to what I should think about for generating those topic 
menus as html docs, feel free to chuck me an email.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* [PATCH] Add some fancy colors in the test library when terminal supports it.
From: Pierre Habouzit @ 2007-10-22  8:13 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---

Maybe this is just me, but I don't find the output of the test-suite
easy to watch while scrolling. This puts some colors in proper places.

  * end-test summaries are in green or red depending on the sucess of
    the tests.
  * errors are in red.
  * skipped tests and other things that tests `say` are in brown (now
    you can _see_ that your testsuite skips some tests on purpose, I
    only noticed recently that I missed part of the environment for
    proper testing).

I'm not 100% sure the test to see if terminal supports color is correct, and
people using emacs shell buffer or alike tools may have better ideas on how to
make it.

and yes, I know that it "depends" upon tput, but if tput isn't available, the
    [ "x$TERM" != "xdumb" ] && tput hpa 60 >/dev/null 2>&1 && tput setaf 1 >/dev/null 2>&1
expression will fail, and color will be disabled.

 t/test-lib.sh |   32 ++++++++++++++++++++++----------
 1 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index cc1253c..c6521c0 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -59,14 +59,24 @@ esac
 # '
 # . ./test-lib.sh
 
+[ "x$TERM" != "xdumb" ] && tput hpa 60 >/dev/null 2>&1 && tput setaf 1 >/dev/null 2>&1
+nocolor=$?
+
+say_color () {
+	[ "$nocolor" = 0 ] &&  [ "$1" != '-1' ] && tput setaf "$1"
+	shift
+	echo "* $*"
+	tput op
+}
+
 error () {
-	echo "* error: $*"
+	say_color 9 "* error: $*"
 	trap - exit
 	exit 1
 }
 
 say () {
-	echo "* $*"
+	say_color 3 "$*"
 }
 
 test "${test_description}" != "" ||
@@ -84,6 +94,8 @@ do
 		exit 0 ;;
 	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
 		verbose=t; shift ;;
+	--no-color)
+	    nocolor=1; shift ;;
 	--no-python)
 		# noop now...
 		shift ;;
@@ -122,13 +134,13 @@ test_tick () {
 
 test_ok_ () {
 	test_count=$(expr "$test_count" + 1)
-	say "  ok $test_count: $@"
+	say_color -1 "  ok $test_count: $@"
 }
 
 test_failure_ () {
 	test_count=$(expr "$test_count" + 1)
 	test_failure=$(expr "$test_failure" + 1);
-	say "FAIL $test_count: $1"
+	say_color 9 "FAIL $test_count: $1"
 	shift
 	echo "$@" | sed -e 's/^/	/'
 	test "$immediate" = "" || { trap - exit; exit 1; }
@@ -158,9 +170,9 @@ test_skip () {
 	done
 	case "$to_skip" in
 	t)
-		say >&3 "skipping test: $@"
+		say_color 10 >&3 "skipping test: $@"
 		test_count=$(expr "$test_count" + 1)
-		say "skip $test_count: $1"
+		say_color 10 "skip $test_count: $1"
 		: true
 		;;
 	*)
@@ -247,11 +259,11 @@ test_done () {
 		# The Makefile provided will clean this test area so
 		# we will leave things as they are.
 
-		say "passed all $test_count test(s)"
+		say_color 2 "passed all $test_count test(s)"
 		exit 0 ;;
 
 	*)
-		say "failed $test_failure among $test_count test(s)"
+		say_color 9 "failed $test_failure among $test_count test(s)"
 		exit 1 ;;
 
 	esac
@@ -296,8 +308,8 @@ do
 	done
 	case "$to_skip" in
 	t)
-		say >&3 "skipping test $this_test altogether"
-		say "skip all tests in $this_test"
+		say_color 10 >&3 "skipping test $this_test altogether"
+		say_color 10 "skip all tests in $this_test"
 		test_done
 	esac
 done
-- 
1.5.3.4.1342.ge0cd-dirty

^ permalink raw reply related

* [PATCH] "git" calls help_unknown_cmd(""); "git help" and "git help -a" return 0
From: Scott R Parish @ 2007-10-22  8:32 UTC (permalink / raw)
  To: git

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 git.c  |    5 ++---
 help.c |    4 ++--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/git.c b/git.c
index 853e66c..e1c99e3 100644
--- a/git.c
+++ b/git.c
@@ -445,9 +445,8 @@ int main(int argc, const char **argv)
 		if (!prefixcmp(argv[0], "--"))
 			argv[0] += 2;
 	} else {
-		/* Default command: "help" */
-		argv[0] = "help";
-		argc = 1;
+		/* The user didn't specify a command; give them help */
+		help_unknown_cmd("");
 	}
 	cmd = argv[0];
 
diff --git a/help.c b/help.c
index 1cd33ec..b0d2dd4 100644
--- a/help.c
+++ b/help.c
@@ -204,14 +204,14 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	if (!help_cmd) {
 		printf("usage: %s\n\n", git_usage_string);
 		list_common_cmds_help();
-		exit(1);
+		exit(0);
 	}
 
 	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a")) {
 		printf("usage: %s\n\n", git_usage_string);
 		if(exec_path)
 			list_commands(exec_path, "git-*");
-		exit(1);
+		exit(0);
 	}
 
 	else
-- 
1.5.3.4.209.g5d1ce-dirty

^ permalink raw reply related

* Re: Howto request: going home in the middle of something?
From: Jan Wielemaker @ 2007-10-22  8:44 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20071018112758.GN18279@machine.or.cz>

Thanks for the replies.	 I think I can live with something like this

	<work, in the middle of something>
	$ git checkout -b home
	$ git commit
	$ git checkout master
	<arriving at home>
	$ git jan@work:repo fetch home:home	(using ssh)
	$ git checkout home
	<continue editing>
	$ git commit --amend
	$ git checkout master
	$ git merge home
	$ git -d home
	$ git commit
	$ git push
	<arriving at work>
	$ git -d home
	$ git pull

Its still a bit many commands and you have to be aware what you are
doing for quite a while, but it does provide one single clean commit
message, doesn't change the shared repo until all is finished and allows
to abandon all work without leaving traces.

Personally I'd be more happy with

	<work, in the middle of something>
	$ git stash
	<arriving at home>
	$ git stash fetch jan@work{0}	(well, some sensible syntax)
	$ git stash apply
	<continue editing>
	$ git commit
	$ git push
	<arriving at work>
	$ git pull

Its not only shorter, but reduces the risc to make mistakes. I think the
missing fetch to copy the stashed data from work to home is actually
there if you know a bit more about git internals. Right? Ideally, this
could be combined in a little command that will simply move the
uncommitted work from one clone to another, provided you have ssh access
to the machine from which you want to fetch the work.

	--- Jan

On Thursday 18 October 2007 13:27, Petr Baudis wrote:
> On Thu, Oct 18, 2007 at 11:44:22AM +0200, Jan Wielemaker wrote:
> > I've somewhere seen it in a mail, but I can't find it anymore. I have a
> > bare central (public) repository and clones on various machines I work
> > on. We all know it, you're right in the middle of something and it is
> > really time to go home. You want to pick up your work at home, but
> > without pushing to the shared repository.
> >
> > I'm sure GIT can do this elegantly, but I'm not yet sure how.  I guess
> > Ideally I want "git stash" at work, transfer the stashed changes to my
> > other machine and apply them.  How do I do that?
> >
> > Alternatively, I guess, one can commit at machine A, fetch the commit
> > from machine A and continue. I'm still too uncertain about the remote
> > access options to work this out properly, but it also feels less
> > clean.
>
>   this should be pretty simple assuming SSH access to machine A. Git can
> fetch over SSH, so it's merely about telling it that repository X is
> available over ssh over there and it'll fetch it home.
>
>   The exact setup depends on whether you want to do this just once or
> semi-regularily.  If the former, just
>
> 	git pull git+ssh://a.machine.aero/absolute/path
>
> Note that this should fetch only the remote master branch, if I'm not
> mistaken.
>
>   If the latter, tell your home repository about your work repository:
>
> 	git remote add workrepo git+ssh://a.machine.aero/absolute/path
>
>   Then, you can anytime just
>
> 	git fetch workrepo
>
> and it will fetch all the branches from workrepo; whether you want to
> use git fetch and git merge or git pull depends on your local
> arrangement of branches at home.
>
>
>   So, basically, when fetching you deal with your work repository
> exactly the same way as in the shared repository.
>
>   When pushing, this is not so trivial. Git _allows_ you to just push to
> your work repository, but if you push to a branch that is currently
> checked out, unexpected things will happen - always avoid that. If you
> can fetch from home at work, do. If not, at least push to a branch at
> work that can never be checked out and is reserved for that purpose.

^ permalink raw reply

* [PATCH] s/pattern/prefix/ in help's list_commands
From: Scott R Parish @ 2007-10-22  8:51 UTC (permalink / raw)
  To: git

list_commands() currently accepts and ignores a "pattern" argument,
and then hard codes a prefix as well as some magic numbers. This
renames the arg from pattern to prefix and uses that instead of the
hardcoded stuff.

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 help.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/help.c b/help.c
index b0d2dd4..950f62d 100644
--- a/help.c
+++ b/help.c
@@ -93,11 +93,12 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 	}
 }
 
-static void list_commands(const char *exec_path, const char *pattern)
+static void list_commands(const char *exec_path, const char *prefix)
 {
 	unsigned int longest = 0;
 	char path[PATH_MAX];
 	int dirlen;
+	int prefix_len = strlen(prefix);
 	DIR *dir = opendir(exec_path);
 	struct dirent *de;
 
@@ -120,7 +121,7 @@ static void list_commands(const char *exec_path, const char *pattern)
 		struct stat st;
 		int entlen;
 
-		if (prefixcmp(de->d_name, "git-"))
+		if (prefixcmp(de->d_name, prefix))
 			continue;
 		strcpy(path+dirlen, de->d_name);
 		if (stat(path, &st) || /* stat, not lstat */
@@ -128,14 +129,14 @@ static void list_commands(const char *exec_path, const char *pattern)
 		    !(st.st_mode & S_IXUSR))
 			continue;
 
-		entlen = strlen(de->d_name);
+		entlen = strlen(de->d_name) - prefix_len;
 		if (has_extension(de->d_name, ".exe"))
 			entlen -= 4;
 
 		if (longest < entlen)
 			longest = entlen;
 
-		add_cmdname(de->d_name + 4, entlen-4);
+		add_cmdname(de->d_name + prefix_len, entlen);
 	}
 	closedir(dir);
 
@@ -143,7 +144,7 @@ static void list_commands(const char *exec_path, const char *pattern)
 	printf("----------------------------");
 	mput_char('-', strlen(exec_path));
 	putchar('\n');
-	pretty_print_string_list(cmdname, longest - 4);
+	pretty_print_string_list(cmdname, longest);
 	putchar('\n');
 }
 
@@ -210,7 +211,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a")) {
 		printf("usage: %s\n\n", git_usage_string);
 		if(exec_path)
-			list_commands(exec_path, "git-*");
+			list_commands(exec_path, "git-");
 		exit(0);
 	}
 
-- 
1.5.3.4.209.g5d1ce-dirty

^ permalink raw reply related

* Re: [PATCH] Add some fancy colors in the test library when terminal supports it.
From: Johannes Sixt @ 2007-10-22  8:53 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Shawn O. Pearce, git
In-Reply-To: <20071022081341.GC32763@artemis.corp>

Pierre Habouzit schrieb:
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
> 
> Maybe this is just me, but I don't find the output of the test-suite
> easy to watch while scrolling. This puts some colors in proper places.
> 
>   * end-test summaries are in green or red depending on the sucess of
>     the tests.
>   * errors are in red.
>   * skipped tests and other things that tests `say` are in brown (now
>     you can _see_ that your testsuite skips some tests on purpose, I
>     only noticed recently that I missed part of the environment for
>     proper testing).
> 
> I'm not 100% sure the test to see if terminal supports color is correct, and
> people using emacs shell buffer or alike tools may have better ideas on how to
> make it.
> 
> and yes, I know that it "depends" upon tput, but if tput isn't available, the
>     [ "x$TERM" != "xdumb" ] && tput hpa 60 >/dev/null 2>&1 && tput setaf 1 >/dev/null 2>&1
> expression will fail, and color will be disabled.
> 
>  t/test-lib.sh |   32 ++++++++++++++++++++++----------
>  1 files changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index cc1253c..c6521c0 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -59,14 +59,24 @@ esac
>  # '
>  # . ./test-lib.sh
>  
> +[ "x$TERM" != "xdumb" ] && tput hpa 60 >/dev/null 2>&1 && tput setaf 1 >/dev/null 2>&1
> +nocolor=$?

test "x$TERM" != "xdumb" &&
	tput hpa 60 >/dev/null 2>&1 &&
	tput setaf 1 >/dev/null 2>&1 &&
	color=t

BTW, doesn't tput fail if stdout/stderr is not a terminal, like above?

> +
> +say_color () {
> +	[ "$nocolor" = 0 ] &&  [ "$1" != '-1' ] && tput setaf "$1"
> +	shift
> +	echo "* $*"
> +	tput op
> +}

What if tput is not available, like on Windows? How about this (at the end 
of the file, so it can obey --no-color):

if test "$color"; then
	say_color () {
		test "$1" != '-1' && tput setaf "$1"
		shift
		echo "* $*"
		tput op
	}
else
	say_color() {
		shift
		echo "* $*"
	}
fi

> +	--no-color)
> +	    nocolor=1; shift ;;

	    color=; shift ;;

-- Hannes "We don't need no double negation"

^ 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