Git development
 help / color / mirror / Atom feed
* Re: What's cooking in git.git (Sep 2011, #06; Wed, 21)
From: Junio C Hamano @ 2011-09-22 16:39 UTC (permalink / raw)
  To: Michael Schubert; +Cc: Carlos Martín Nieto, git
In-Reply-To: <4E7AFC6C.7080603@elegosoft.com>

Michael Schubert <mschub@elegosoft.com> writes:

> On 09/22/2011 10:37 AM, Carlos Martín Nieto wrote:
>> On Wed, 2011-09-21 at 22:04 -0700, Junio C Hamano wrote:
>>> * cn/eradicate-working-copy (2011-09-21) 2 commits
>>>  - patch-id.c: use strbuf instead of a fixed buffer
>>>  - Remove 'working copy' from the documentation and C code
>> 
>> It looks like that first commit sneaked in there. Shouldn't that be its
>> own topic?
>
> It's in pu twice:

Thansk for noticing. Will remove.

> On 09/22/2011 07:04 AM, Junio C Hamano wrote: 
>> * cn/eradicate-working-copy (2011-09-21) 2 commits
>>  - patch-id.c: use strbuf instead of a fixed buffer
>>  - Remove 'working copy' from the documentation and C code
>
>> * ms/patch-id-with-overlong-line (2011-09-21) 1 commit
>>  - patch-id.c: use strbuf instead of a fixed buffer
>
> 64128da and a6c5c60
>
> There's also a minor typo in the last sentence of the commit message.
> Should I resend?

If that is s/user/use/ and nothing else, I can amend locally; otherwise
please do.

The last part of the "rather dumb heiristic" you talk about is designed to
parse "cmd | diff-tree -p --stdin" output. I agree with you that it can
safely tightened a bit by checking that 41st byte is the EOL, but it
probably is not worth it.

^ permalink raw reply

* Re: Bug: git log --numstat counts wrong
From: René Scharfe @ 2011-09-22 16:15 UTC (permalink / raw)
  To: git; +Cc: git
In-Reply-To: <7vobyd1vmo.fsf@alter.siamese.dyndns.org>

Am 21.09.2011 22:35, schrieb Junio C Hamano:
> Alexander Pepper <pepper@inf.fu-berlin.de> writes:
> 
>> Am 21.09.2011 um 14:24 schrieb Junio C Hamano:
>>>> $ git log --numstat 48a07e7e533f507228e8d1c99d4d48e175e14260
>>>> [...]
>>>> 11      10      src/java/voldemort/server/storage/StorageService.java
>>>
>>> Didn't we update it this already? I seem to get 10/9 here not 11/10.
>>
>> Current 'maint' (cd2b8ae9), 'master' (4b5eac7f)...
> 
> That's a tad old master you seem to have.
> 
> Strangely, bisection points at 27af01d5523, which was supposed to be only
> about performance and never about correctness. There is something fishy
> going on....

The patch below reverts a part of 27af01d5523 that's not explained in its
commit message and doesn't seem to contribute to the intended speedup.  It
seems to restore the original diff output.  I don't know how it's actually
doing that, though, as I haven't dug into the code at all.

Alexander, can you confirm that this patch restores the old behaviour of
git diff and git show for your test cases?

Ray, are you able to write a commit message for this patch if it turns out
to be useful?

René


diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
index 5a33d1a..e419f4f 100644
--- a/xdiff/xprepare.c
+++ b/xdiff/xprepare.c
@@ -383,7 +383,7 @@ static int xdl_clean_mmatch(char const *dis, long i, long s, long e) {
  * might be potentially discarded if they happear in a run of discardable.
  */
 static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xdf2) {
-	long i, nm, nreff;
+	long i, nm, nreff, mlim;
 	xrecord_t **recs;
 	xdlclass_t *rcrec;
 	char *dis, *dis1, *dis2;
@@ -396,16 +396,20 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
 	dis1 = dis;
 	dis2 = dis1 + xdf1->nrec + 1;
 
+	if ((mlim = xdl_bogosqrt(xdf1->nrec)) > XDL_MAX_EQLIMIT)
+		mlim = XDL_MAX_EQLIMIT;
 	for (i = xdf1->dstart, recs = &xdf1->recs[xdf1->dstart]; i <= xdf1->dend; i++, recs++) {
 		rcrec = cf->rcrecs[(*recs)->ha];
 		nm = rcrec ? rcrec->len2 : 0;
-		dis1[i] = (nm == 0) ? 0: 1;
+		dis1[i] = (nm == 0) ? 0: (nm >= mlim) ? 2: 1;
 	}
 
+	if ((mlim = xdl_bogosqrt(xdf2->nrec)) > XDL_MAX_EQLIMIT)
+		mlim = XDL_MAX_EQLIMIT;
 	for (i = xdf2->dstart, recs = &xdf2->recs[xdf2->dstart]; i <= xdf2->dend; i++, recs++) {
 		rcrec = cf->rcrecs[(*recs)->ha];
 		nm = rcrec ? rcrec->len1 : 0;
-		dis2[i] = (nm == 0) ? 0: 1;
+		dis2[i] = (nm == 0) ? 0: (nm >= mlim) ? 2: 1;
 	}
 
 	for (nreff = 0, i = xdf1->dstart, recs = &xdf1->recs[xdf1->dstart];

^ permalink raw reply related

* Re: How to use git attributes to configure server-side checks?
From: Jay Soffian @ 2011-09-22 15:41 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Junio C Hamano, git discussion list
In-Reply-To: <4E7AF1AE.5030005@alum.mit.edu>

On Thu, Sep 22, 2011 at 4:28 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> Where does "git show --check" read its gitattributes (i.e.,
> "whitespace") from?

It looks like builtin_checkdiff (diff.c) calls whitespace_rule (ws.c)
which in turn calls git_check_attr (attr.c) which, in a bare repo,
considers $(prefix)/etc/gitattributes, core.attributesfile and
$GIT_DIR/info/attributes, falling back to the default whitespace rule
of trailing_space, space_before_tab, and tab-width of 8
(WS_DEFAULT_RULE in cache.h).

Thank you for this thread. I was under the illusion that diff-tree
--check considered in-tree .gitattributes, but the code seems to
indicate otherwise. :-(

j.

^ permalink raw reply

* Re: [PATCH/RFCv4 2/4] gitweb: Add manpage for /etc/gitweb.conf (for gitweb documentation)
From: Jakub Narebski @ 2011-09-22 13:41 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, Drew Northup
In-Reply-To: <20110920044118.GI6343@elie>

On Tue, 20 Sep 2011, Jonathan Nieder wrote:
> Jakub Narebski wrote:
> 
> > [jn: Improved, extended, removed duplicate info from README]
> 
> Hoorah!  I have very little to add to what's already been done ---
> mostly I can just lend my ear as a native English speaker.

Thanks, that is a lot of help; English is not my primary language.

> > +The syntax of the configuration files is that of Perl, as these files are
> > +indeed handled as fragments of Perl code (the language that gitweb itself is
> > +written in). Variables may be set using "`our $variable = value`"; text from
> > +"#" character until the end of a line is ignored. See the *perlsyn*(1) man
> > +page for more information.
> > +
> > +Actually using `our` qualifier in `our $variable = <value>;` is a safety
> > +check: if newer gitweb does no longer use given variable, by using `our` we
> > +won't get syntax errors.
> 
> Language nit: for "indeed", something like "internally" might be more
> natural.
> 
> The syntax and the typical idiom were already described in the
> DESCRIPTION section, so the emphasis should be somehow different here
> to avoid boring the reader.  One way might be to plunge directly into
> what the second paragraph above says:
> 
>  The syntax of the configuration files is that of Perl, since these files are
>  handled by sourcing them as fragments of Perl code (the language that
>  gitweb itself is written in). Variables are typically set using the
>  `our` qualifier (as in "`our $variable = <value>;`") to avoid syntax
>  errors if a new version of gitweb no longer uses a variable and therefore
>  stops declaring it.

Thanks, I used your proposal.  It reduces duplication, and reads better.
 
> > +
> > +You can include other configuration file using read_config_file()
> > +subroutine.  For example, you can read defaults in fallback
> > +system-wide GITWEB_CONFIG_SYSTEM from GITWEB_CONFIG by adding
> > +
> > +  read_config_file($GITWEB_CONFIG_SYSTEM);
> > +
> > +at very beginning of per-instance GITWEB_CONFIG file.  In this case
> > +settings in said per-instance file will override settings from
> > +system-wide configuration file.  Note that read_config_file checks
> > +itself that the file it reads exists.
[...]

> The reader wonders: After checking that the file exists, what does
> read_config_file do (does it silently skip it or error out)?  And why
> would I use this trick instead of just writing a gitweb-common.conf
> file?  (Was read_config_file introduced before gitweb-common.conf,
> making this useful when creating a configuration that should be usable
> with older versions of gitweb?)

I have replaced it now with the following:

  You can include other configuration file using read_config_file()
  subroutine.  For example, one might want to put gitweb configuration
  related to access control for viewing repositories via Gitolite (one
  of git repository management tools) in a separate file, e.g. in
  '/etc/gitweb-gitolite.conf'.  To include it, put
  
  --------------------------------------------------
  read_config_file("/etc/gitweb-gitolite.conf");
  --------------------------------------------------
  
  somewhere in gitweb configuration file used, e.g. in per-installation
  gitweb configuration file.  Note that read_config_file() checks itself
  that the file it reads exists, and does nothing if it is not found.
  It also handles errors in included file.

> > +$project_maxdepth::
> > +	Filesystem traversing depth limit for recursively scanning for git
> > +	repositories, used if `$projects_list` (below) is unset.  The default
> > +	value of this variable is determined by build-time configuration
> > +	variable `GITWEB_PROJECT_MAXDEPTH`, which defaults to 2007.
> 
> "Filesystem traversing depth limit" is quite a long stack of nouns :),
> and unfortunately I don't know what it means.  Is this the depth of
> nested subdirectories below $project_root that gitweb will look at
> when discovering repositories?  If I have no subdirectories in the
> projectroot except the repositories themselves, should I set this to 1
> or 0?  What happens with forks?  Is this just a convenience feature or
> can it be used for security or to create performance gaurantees?
> 
> By the way, what happens if projectroot contains a symlink to some
> directory elsewhere in the filesystem containing repositories --- will
> gitweb traverse it?

I have changed order of describing variables, putting $projects_list
before $project_maxdepth - this way we don't have to refer to following
text.

I have replaced the description of $project_maxdepth with the following

  $project_maxdepth::
  	If `$projects_list` variable is unset, gitweb will recursively
  	scan filesystem for git repositories.  The `$project_maxdepth`
  	is used to limit traversing depth, relative to `$projectroot`
  	(starting point); it means that directories which are further
  	from `$projectroot` than `$project_maxdepth` will be skipped.
  +
  It is purely performance optimization, originally intended for MacOS X,
  where recursive directory traversal is slow.  Gitweb follows symbolic
  links, but it detects cycles, ignoring any duplicate files and directories.
  +
  The default value of this variable is determined by the build-time
  configuration variable `GITWEB_PROJECT_MAXDEPTH`, which defaults to
  2007.
 
I hope that it makes meaning and use of this variable more clear.

> What if I want this to be infinite (i.e., to disable the feature) ---
> would I be crazy?

It is currently not possible to disable this feature.  The value of 2007
should be enough in most cases (read: all sane cases).
 
> > +$projects_list::
> > +	Plain text file listing projects or name of directory
> > +	to be scanned for projects.
[...]

> When this is a relative path, what is it taken relative to?

As with all relative paths to files, it is relative to where CGI script
is.  I probably should write about this somewhere...

> > +
> > +$export_ok::
> > +	Show repository only if this file exists (in repository).  Only
> > +	effective if this variable evaluates to true.  Can be set during
> > +	building gitweb via `GITWEB_EXPORT_OK`.  [No default / Not set]
> 
> This is always a relative path, right?  What is it relative to ---
> $GIT_DIR, I guess?
> 
> Usage nits: the phrase starting with "during" reads more naturally if
> "during" is replaced with "when" and "via" replaced with "by setting".
> If there were no default, that would mean that gitweb errors out when
> gitweb.conf does not set this variable; instead, the default seems to
> be "undef" (or 'false') if I understand correctly.

Description of $export_ok got replaced by the following:

  $export_ok::
  	Show repository only if this file exists (in repository).  Only
  	effective if this variable evaluates to true.  Can be set when
  	building gitweb by setting `GITWEB_EXPORT_OK`.  This path is
  	relative to `GIT_DIR`.  git-daemon[1] uses 'git-daemon-export-ok',
  	unless started with `--export-all`.  [No default / Not set
  	(this feature is turned off)]

> > +
> > +$export_auth_hook::
> > +	Show repository only if this subroutine returns true, when given as
> > +	the only parameter the full path to the project.  Example:
> > ++
> > +----------------------------------------------------------------------------
> > +our $export_auth_hook = sub { return -e "$_[0]/git-daemon-export-ok"; };
> > +----------------------------------------------------------------------------
> > ++
> > +though the above might be done by using `$export_ok` instead
> > ++
> > +----------------------------------------------------------------------------
> > +our $export_ok = "git-daemon-export-ok";
> > +----------------------------------------------------------------------------
> 
> Style nit: commands in this kind of documentation should be directed to
> the reader, not gitweb, so it would be nicer to explain this in terms of
> what the $export_auth_hook is.

Thanks.  I should probably rewrite description of $export_ok in the same
way, isn't it?

>                                 That is, something like the following: 
> 
>  $export_auth_hook::
> 	Function used to determine which repositories should be shown.
> 	This subroutine should take one parameter, the full path to
> 	a project, and if it returns true, that project will be included
> 	in the projects list and can be accessed through gitweb as long
> 	as it fulfills the other requirements described by $export_ok,
> 	$projects_list, and $projects_maxdepth.  Example:

Thanks, I have used your description.
 
> Is "our $export_auth_hook = undef;" a valid configuration?  What is the
> default?

I have added the following line at the end of description of this
variable:

  If not set (default), it means that this feature is disabled.

> > +
> > +$strict_export::
> > +	Only allow viewing of repositories also shown on the overview page.
> > +	This for example makes `$gitweb_export_ok` file decide if repository is
> > +	available and not only if it is shown.  If `$gitweb_list` points to
> > +	file with list of project, only those repositories listed would be
> > +	available for gitweb.  Can be set during building gitweb via
> > +	`GITWEB_STRICT_EXPORT`.  [No default / Not set]
> 
> Is the default behavior as though this were true or false?

Last sentence now reads:

  [No default / Not set (you can access	repositories hidden from projects
  list page)]

> > +Finding files
> > +~~~~~~~~~~~~~
> > +Those configuration variables tell gitweb where to find files.  Values of
> > +those variables are paths on filesystem.  Of those only `$GIT` is required
> > +to be (correctly) set for gitweb to be able to work; all the rest are
> > +required only for extra configuration or extra features.
[...]
> When you say the remainder are only required in special cases, does
> that mean that they are ignored unless some other enabling variable is
> set or that they can be set to "undef" to disable the relevant
> feature?

I have removed this sequence, and extended description of each variable
instead.
 
> > +$mimetypes_file::
> > +	File to use for (filename extension based) guessing of MIME types before
> > +	trying '/etc/mime.types'.  Path, if relative, is taken currently as
> > +	relative to the current git repository.  [Unset by default]

> The use of "currently" sounds like an apology.  Does that mean that
> gitweb ought to be rejecting relative paths for this variable?

I think it was originally intended as note that this resolution of relative
path is subject to change in future versions of gitweb.  But I think it
is not true (backward compatibility), and it is more important that this
resolution of relative path is untypical.

I have reformulated this description to read:

  $mimetypes_file::
  	File to use for (filename extension based) guessing of MIME types before
  	trying '/etc/mime.types'.  *NOTE* that this path, if relative, is taken
  	as relative to the current git repository, not to CGI script.  If unset,
  	only '/etc/mime.types' is used (if present on filesystem).  If no mimetypes
  	file is found, mimetype guessing bassed on extension of file is disabled.
  	[Unset by default]
 
> > +
> > +$highlight_bin::
> > +	Path to the highlight executable to use (must be the one from
> > +	http://www.andre-simon.de due to assumptions about parameters and output).
> > +	Useful if 'highlight' is not installed on your webserver's PATH.
> > +        [Default: 'highlight']
> > ++
> > +*NOTE*: if you want to add support for new syntax (supported by
> > +"highlight" but not used by gitweb), you need to modify `%highlight_ext`
> > +or `%highlight_basename`, depending on whether you detect type of file
> > +based on extension (for example "*.sh") or on its basename (for example
> > +"Makefile").  Keys of those hashes are extension or basename,
> > +respectively, and value for given key is name of syntax to be passed via
> > +`--syntax <syntax>` to highlighter.
[...]

> Is "*.sh" actually an example of an extension?  I.e., do I write the
> following?
> 
> 	our %highlight_ext;
> 	$highlight_ext{"*.sh"} = "sh";

You are right, it should be "sh", not "*.sh".  Fixed, and added example
(about "phtml", which usually means PHP, but might mean embedded Perl
(ePerl)).

> > +Links and their targets
> > +~~~~~~~~~~~~~~~~~~~~~~~
> > +Configuration variables described below configure some of gitweb links:
> > +their target and their look (text or image), and where to find page
> > +prerequisites (stylesheet, favicon, images, scripts).  Usually they are left
> > +at their default values, with the possible exception of `@stylesheets`
> > +variable.
> 
> Language nits: missing "The" before "Configuration variables described
> below"; missing "some" before "supporting files".
                                ^^^^^^^^^^^^^^^^^^---???  
 
> > +
> > +@stylesheets::
> > +	List of URIs of stylesheets (relative to base URI of a page). You
> > +	might specify more than one stylesheet, for example use gitweb.css
> > +	as base, with site specific modifications in separate stylesheet
> > +	to make it easier to upgrade gitweb. You can add a `site` stylesheet
> > +	for example by putting
> > ++
> > +----------------------------------------------------------------------------
> > +push @stylesheets, "gitweb-site.css";
> > +----------------------------------------------------------------------------
> > ++
> > +in the gitweb config file.  Those values that are relative paths, are
> > +relative to base URI of gitweb.
> 
> [...]  What is a base URI --- is it set by another variable?

Base URI is a concept defined in HTML / XHTML specification (or in URI
definition), so I'd rather not explain it here.


> > +This list should contain URI of gitweb's stylesheet.  The URI of gitweb
> > +stylesheet is set during build time via `GITWEB_CSS` variable.  It's default
> > +value is 'static/gitweb.css' (or 'static/gitweb.min.css' if the `CSSMIN`
> > +variable is defined, i.e. if CSS minifier is used during build).
> > ++
> > +*Note*: there is also legacy `$stylesheet` configuration variable, which was
> > +used by older gitweb.
[...]
> If I am new on the job and find the $stylesheet variable set, what
> should I interpret it to mean?  How can it be translated to the newer
> @stylesheets style?  What happens if both variables are set --- does
> one override the other, or are they combined somehow?

Added

  If `$stylesheet` variable is defined, only CSS stylesheet
  given by this variable is used by gitweb.

I hope that old config files that define this variable are rare.

> > +$home_link::
> > +	Target of the home link on top of all pages (the first part of view
> > +	"breadcrumbs").  By default set to absolute URI of a page ($my_uri).

> Which page does "a page" refer to here (doesn't $my_uri change from
> request to request)?

I have extended last sequence of this description to read:

  By default it is set to the absolute URI of a current page
  (to the value of `$my_uri` variable, or to "/" if `$my_uri` is undefined
  or is an empty string).
 
$my_uri changes from request to request, and $home_link too.

By default config is re-read on each request; if not, one should
use $per_request_config subroutine to set this variable if it changes
from request to request.

> > +$default_projects_order::
> > +	Default value of ordering of projects on projects list page.  Valid
> > +	values are "none" (unsorted), "project" (by project name, i.e. path
> > +	to repository relative to `$projectroot`), "descr" (project
> > +	description), "owner", and "age" (by date of most current commit).
> > ++
> > +Default value is "project".  Unknown value means unsorted.
> 
> What does "Default" mean here (i.e., what overrides it)?

Added

  [...], which means the ordering used if you don't explicitly sort
  projects list (if there is no "o" CGI query parameter in the URL).
 
> > +
> > +
> > +Changing gitweb behavior
> > +~~~~~~~~~~~~~~~~~~~~~~~~
> > +Those configuration variables control _internal_ gitweb behavior.
> 
> [...] Does internal behavior mean "functionality as opposed to
> appearance and input location" or something like that?

Yes, it does.

> > +
> > +$default_blob_plain_mimetype::
> > +	Default mimetype for blob_plain (raw) view, if mimetype checking
> > +	doesn't result in some other type; by default "text/plain".
[...]

> Where can I look to read more about mimetype autodetection?

Added the following sentence:

  	Gitweb guesses mimetype of a file to display based on extension
  	of its filename, using `$mimetypes_file` (if set and file exists)
  	and '/etc/mime.types' files (see *mime.types*(5) manpage; only
  	filename extension rules are supported by gitweb).
 
> > +@diff_opts::
> > +	Rename detection options for git-diff and git-diff-tree. By default
> > +	(\'-M'); set it to (\'-C') or (\'-C', \'-C') to also detect copies,
> > +	or set it to () i.e. empty list if you don't want to have renames
> > +	detection.
[...]

> Probably worth mentioning that GNU patch still has problems with some
> rename patches, especially when they involve file copies ['-C'] or
> criss-cross renames ['-B'] (see [1] and [2], for example).
> 
> [1] http://savannah.gnu.org/bugs/?31058
> [2] http://thread.gmane.org/gmane.linux.ports.sh.devel/8697/focus=8773

I have added the following paragraph:

  +
  *Note* that rename and especially copy detection can be quite
  CPU-intensive.  Note also that non git tools can have problems with
  patches generated with options mentioned above, especially when they
  involve file copies (\'-C') or criss-cross renames (\'-B').


> > +@git_base_url_list::
> > +	List of git base URLs used for URL to where fetch project from, shown
> > +	in project summary page.  Full URL is "`$git_base_url/$project`".  You
> > +	can setup multiple base URLs (for example one for `git://` protocol
> > +	access, and one for `http://` "dumb" protocol access).  Note that per
> > +	repository configuration can be set in '$GIT_DIR/cloneurl' file, or as
> > +	values of multi-value `gitweb.url` configuration variable in project
> > +	config.
> > ++
> > +You can setup one single value (single entry/item in this list) during
> > +building by using `GITWEB_BASE_URL` built-time configuration variable.
> > +[No default]
> 
> Language nits: "git base URLs used for URL to where fetch project from"
> means something like "Git URLs relative to which projects can be
> fetched"; [...]

[...]
> Is the per repository cloneurl added to this list, or does it override
> it?

I have rewritten it to hopefully more clear version, and added information
about precedence of cloneurl and the like:

  @git_base_url_list::
  	List of git base URLs.  These URLs are used to generate URLs
  	describing from where to fetch a project, which are shown on
  	project summary page.  The full fetch URL is "`$git_base_url/$project`",
  	for each element of this list. You can set up multiple base URLs
  	(for example one for `git://` protocol, and one for `http://`
  	protocol).
  +
  Note that per repository configuration can be set in '$GIT_DIR/cloneurl'
  file, or as values of multi-value `gitweb.url` configuration variable in
  project config.  Per-repository configuration takes precedence over value
  composed from `@git_base_url_list` elements and project name.
  +
  You can setup one single value (single entry/item in this list) at build
  time by setting the `GITWEB_BASE_URL` built-time configuration variable.
  [Default: (), i.e. empty list]

> > +$maxload::
> > +	Used to set the maximum load that we will still respond to gitweb queries.
> > +	If server load exceed this value then return "503 Service Unavailable"
> > +	error. Server load is taken to be 0 if gitweb cannot determine its value.
> > +	Set it to undefined value to turn it off. [Default: 300]
> 
> Probably worth spelling out that this is a "load average" (as shown by uptime(1)).

I have added the following sentence to describe it:

  Currently it works only on Linux,
  where it uses '/proc/loadavg'; the load there is the number of active
  tasks on the system -- processes that are actually running -- averaged
  over the last minute.
 

> > +$per_request_config::
> > +	If set to code reference, it would be run once per each request.  You can
> > +	set parts of configuration that change per session, e.g. by adding
> > +	the following code to gitweb configuration file
> > ++
> > +--------------------------------------------------------------------------------
> > +our $per_request_config = sub {
> > +	$ENV{GL_USER} = $cgi->remote_user || "gitweb";
> > +};
> > +--------------------------------------------------------------------------------
> 
> Language nits: missing "this is" before "set to a code reference";
> "would" should be "will" (those crazy English conditionals again); the
> appropriate preposition before "each request" is "for", not "per";
> missing "this way" after "change per session"; would be clearer with a
> full stop before the example, and with the example made into a full
> sentence ("For example, one might use the following code in a gitweb
> configuration file:").
> 
> > ++
> > +Otherwise it is treated as boolean value: if true gitweb would process
> > +config file once per request, if false it would process config file only
> > +once.  [Default: true]
> > ++
> > +*NOTE*: `$my_url`, `$my_uri`, and `$base_url` are overwritten with their default
> > +values before every request, so if you want to change them, be sure to set
> > +this variable to true or a code reference effecting the desired changes.
> 
> At this point, I've forgotten what the "Otherwise" is contrasting with;
> probably best to repeat it ("If $per_request_config is not a code
> reference, it is interpreted as a boolean value."); "would" should be
> "will" again (2x); missing "the" or "each" before "config file" (2x);
> missing "and" before "if it is false"; "it" could be "gitweb" to
> avoid confusion in this pronoun-heavy sentence; missing "each time it
> is executed" after "process the config file only once".
> 
> Probably worth mentioning that this variable starts to shine when one
> gitweb instance is used to serve multiple requests, with CGI
> replacements like mod_perl, fastcgi, plackup, and so on.

I have rewritten and extended it as following:

  If `$per_request_config` is not a code reference, it is interpreted as boolean
  value.  If it is true gitweb will process config files once per request,
  and if it is false gitweb will process config files only once, each time it
  is executed.  [Default: true]
  +
  *NOTE*: `$my_url`, `$my_uri`, and `$base_url` are overwritten with their default
  values before every request, so if you want to change them, be sure to set
  this variable to true or a code reference effecting the desired changes.
  +
  This variable matters only when using persistent web environments that
  serve multiple requests using single gitweb instance, like mod_perl,
  FastCGI or Plackup.

> > +$version::
> > +	Gitweb version, set automatically when creating gitweb.cgi from
> > +	gitweb.perl. You might want to modify it if you are running modified
> > +	gitweb, for example 
> > ++
> > +---------------------------------------------------
> > +our $version .= " with caching";
> > +---------------------------------------------------
> 
> Might be worth mentioning this is only used in HTML comments and the
> "generator" metadata field, nothing more special than that.

Added:

  This variable is purely informational, used e.g. in the "generator"
  meta header in HTML header.
 
> > +
> > +$my_url::
> > +$my_uri::
> > +	Full URL and absolute URL of gitweb script;
> > +	in earlier versions of gitweb you might have need to set those
> > +	variables, now there should be no need to do it.  See
> > +	`$per_request_config` if you need to set them still.
[...]

> What is the difference between a full URL and an absolute URL?

Full URL is e.g. 'http://git.example.com/gitweb.cgi', while absolute URL
is e.g. '/gitweb.cgi'.

> > +sub::
> > +	Subroutine that will be called with default options as parameters to
> > +	examine per-repository configuration, but only if feature is
> > +	overridable (\'override' is set to true).  If not present then
> > +	per-repository override for given feature is not supported.
> > ++
> > +You wouldn't need to ever change it in gitweb config file.
> 
> Language nits: "default options as parameters" means "the array
> referred to by the 'default' field as parameter list", I guess.
> Missing "the" before "feature is overridable" and "gitweb
> configuration files".  "given feature" -> "this feature".  "not
> supported" -> "not enabled", maybe.
> 
> Actually, I'm not sure I understand this one.  Are users supposed to
> set this field?  Where can they look to find out what features have it
> set by default?  Who calls this function, and what does that person or
> code path do with the return value?  If it is not part of the gitweb
> configuration that an administrator might modify (the subject of this
> manpage), why not just say that ("subroutine used internally; present
> if and only if this feature can be made overridable with the
> "override" field; you should not change it") and leave it at that?

Changed this to:

  sub::
  	Internal detail of implementation.  What is important is that
  	if this field is not present then per-repository override for
  	given feature is not supported.
  +
  You wouldn't need to ever change it in gitweb config file.
 
> > +
> > +
> > +Features in `%feature`
> > +~~~~~~~~~~~~~~~~~~~~~~
> > +Below there are described gitweb features that are configurable via
> > +`%feature` hash.  For a complete list please consult gitweb sources.
> 
> Language nit: "Below there are described gitweb features ..." -> "The
> gitweb features ... are listed below".  The second sentence could
> probably be more apologetic, something like "Currently the only
> authoritative and complete list is in the comments in the gitweb.cgi
> source code."

Reformulated it to read:

  The gitweb features that are configurable via `%feature` hash are listed
  below.  This should be a complete list, but ultimately the authoritative
  and complete list is in gitweb.cgi source code, with features described
  in the comments.

> > +pickaxe::
> > +	Enable the so called pickaxe search, which will list the commits
> > +	that modified a given string in a file.  This can be practical and
> > +	quite faster alternative to "blame" action, but still potentially
> > +	CPU-intensive.  Enabled by default.
> > ++
> > +The pickaxe search is described in linkgit:git-log[1] (the
> > +description of `-S<string>` option, which refers to pickaxe entry in
> > +linkgit:gitdiffcore[7] for more details).
> > ++
> > +This feature can be configured on a per-repository basis via
> > +repository's `gitweb.pickaxe` configuration variable (boolean).
> 
> I think "modified" means "introduced or removed" (as an approximation to
> "changes the number of occurences of").

It is even more correct with respect to mechanism behind '-S' option.

> (By the way, is there any way for the the very paranoid to limit the
> length of regexes used or CPU time used per request, to get some
> reasonable cap on this sort of thing?  I guess that's more in the
> realm of web server configuration than something gitweb should be
> responsible for --- but if there's some common practice or well known
> reference on the topic, it could be worth mentioning at the top of
> this GITWEB FEATURES section some day.)

I don't know if it is possible to limit CPU time used per request.
I think most of time is spent in git commands; gitweb currently doesn't
offer any way to renice or limit time of running git commands.  The
only load-related mechanism is $maxload.

> > +show-sizes::
> > +	Enable showing size of blobs (ordinary files) in a "tree" view, in a
> > +	separate column, similar to what `ls -l` does; see description of
> > +	`-l` option in linkgit:git-ls-tree[1] manpage.  This cost a bit of
> > +	I/O.  Enabled by default.
> > ++
> > +This feature can be configured on a per-repository basis via
> > +repository's `gitweb.showsizes` configuration variable (boolean).
> 
> Nit: "cost" -> "costs".  Interesting (I guess packv4 will help with
> the CPU [decompression] cost of this when it comes).

Nb. the fast that directory entries ('tree' objects) do not have their
size displayed was result of request from people working on packv4, where
'tree' objects would be stored decomposed, and calculating object size
for a 'tree' object will be quite costly.

Add to that the fact that 'tree' object size is not very interesting...
 
> > +avatar::
> > +	Avatar support.  When this feature is enabled, views such as
> > +	"shortlog" or "commit" will display an avatar associated with
> > +	the email of the committer(s) and/or author(s).
> > ++
> > +Currently available providers are *"gravatar"* and *"picon"*.
> > +If an unknown provider is specified, the feature is disabled.
> > +*Note* that some provides might require extra Perl packages to be
> > +installed; see 'gitweb/INSTALL' for more details.
[...]

> If I set 'default' to ["gravator", "picon", "unknown"], will that
> really disable the feature?

Only one provider at a time can be selected; I have added this
information to description.
 

> > +forks::
> > +	Make gitweb consider projects in subdirectories of project root
> > +	(basename) to be forks of existing projects.  Given project
> > +	`$projname.git`, projects matching `$projname/*.git` will not be
> > +	shown in the main projects list, instead a \'+' mark will be added
> > +	to `$projname` there and a "forks" view will be enabled for the
> > +	project, listing all the forks.  If project list is taken from a
> > +	file, forks have to be listed after the main project.
> > ++
> > +Project specific override is not supported.
[...]

> If I understand correctly, "Given project `$projname.git`, projects
> matching `$projname/*.git`" can be written as "If the project
> `$projname.git` exists, projects in the `$projname/` directory".  Is
> it just projects in that directory, or are subdirectories included, as
> in the following example?
> 
> 	project.git
> 	project/foo/bar/baz/qux.git

Yes, they are.

The new improved and extended version is now:

  forks::
  	If this feature is enabled, gitweb considers projects in
  	subdirectories of project root (basename) to be forks of existing
  	projects.  For each project `$projname.git`, projects in the
  	`$projname/` directory and its subdirectories will not be
  	shown in the main projects list.  Instead, a \'+' mark is shown
  	next to `$projname`, which links to a "forks" view that lists all
  	the forks (all projects in `$projname/` subdirectory).  Additionally
  	a "forks" view for a project is linked from project summary page.
  +
  If the project list is taken from a file (`$projects_list` points to a
  file), forks are only recognized if they are listed after the main project
  in that file.
  +
  Project specific override is not supported.

> > +
> > +actions::
> > +	Insert custom links to the action bar of all project pages.  This
> > +	enables you to link to third-party scripts integrating into gitweb.
> > ++
> > +The "default" value consists of a list of triplets in the form
> > +`("<label>", "<link>", "<position>")` where "position" is the label
> > +after which to insert the link, "link" is a format string where `%n`
> > +expands to the project name, `%f` to the project path within the
> > +filesystem, `%h` to the current hash (\'h' gitweb parameter) and
> > +`%b` to the current hash base (\'hb' gitweb parameter); `%%` expands
> > +to \'%'.
> 
> Interesting.  "enables" should be "allows" here.

Right.
 
> Is %f an absolute path (i.e., starting with '/')?
 
It is "$projectroot/$project" (which I have added to this description).

> > +In most case you would want to change only default timezone:
> > ++
> > +---------------------------------------------------------------------------
> > +$feature{'javascript-timezone'}{'default'}[0] = "utc";
> > +---------------------------------------------------------------------------
[...]
> > +Timezone value can be "local" (for local timezone that browser uses), "utc"
> > +(what gitweb uses when JavaScript or this feature is disabled), or numerical
> > +timezone in the form of "+/-HHMM" for example "+0200".  This way is
> > +guaranteed to be backward compatibile.
> > ++
> > +Project specific override is not supported.
> 
> Language nits: when we are talking about supported values in general,
> it sounds better for some reason with "value" -> values"; "numerical
> timezone" -> "numerical timezones"; "for example" -> "such as".
> 
> What is "This way", and what other way should people be avoiding to
> prevent forward compatibility gotchas?
 
"This way" refers to the example configuration.  I have reformulated
this description by removing last sentence, and adding

  +
  The example configuration presented here is guaranteed to be backwards
  and forward compatible.

just after example.


> [...]
> > +GITWEB_CONFIG::
> > +	Sets location of per-instance configuration file.
> [...]
> > +FILES
> > +-----
> > +gitweb_config.perl::
> > +	This is default value for per-instance configuration file.  The
> > +	format of this file is described above.
> > +/etc/gitweb.conf::
> > +	This is default value for fallback system-wide configuration
> > +	file.  This file is used only if per-instance configuration
> > +	variable is not found.
> > +/etc/gitweb-common.conf::
> > +	This is default value for common system-wide configuration
> > +	file.
> 
> "default value" sounds strange here --- I guess I would have expected
> something more like
> 
>  ENVIRONMENT
>  -----------
>  ...
>  GITWEB_CONFIG::
> 	Path to use to find the per-instance configuration file,
> 	instead of gitweb_config.perl.  If relative, this is relative
> 	to $GITWEBDIR.
> 
>  FILES
>  -----
>  /etc/gitweb-common.conf::
> 	Options to be shared by all gitweb instances.  The format is
> 	described above.
>  $GITWEBDIR/gitweb_config.perl::
> 	Additional settings for a particular gitweb instance (in
> 	the same format).
>  /etc/gitweb.conf::
> 	Fallback configuration file with settings for gitweb instances
> 	that do not contain a gitweb_config.perl file.

I'll think about this proposal.

For now I have just changed "default value for ..." to
"default name of ...".
  

Thank you very much for your feedback.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: Bug: git log --numstat counts wrong
From: Alexander Pepper @ 2011-09-22 13:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Tay Ray Chuan
In-Reply-To: <7vobyd1vmo.fsf@alter.siamese.dyndns.org>

Am 21.09.2011 um 22:35 schrieb Junio C Hamano:
> That's a tad old master you seem to have.
Up until now I only followed the github clone, but that seems to be dated. Since kernel.org is still down, I'll now follow the google code clone.


Am 21.09.2011 um 22:35 schrieb Junio C Hamano:
>> Am 21.09.2011 um 14:24 schrieb Junio C Hamano:
>>>> $ git log --numstat 48a07e7e533f507228e8d1c99d4d48e175e14260
>>>> [...]
>>>> 11      10      src/java/voldemort/server/storage/StorageService.java
>>> 
>>> Didn't we update it this already? I seem to get 10/9 here not 11/10.
>> 
>> Current 'maint' (cd2b8ae9), 'master' (4b5eac7f)...
> 
> Strangely, bisection points at 27af01d5523, which was supposed to be only
> about performance and never about correctness. There is something fishy
> going on....
I also did some tests, and besides --numstat also git show sometimes show different patches in comparison to older git versions. The last version with the "old" git show output is 1.7.7.rc0 and the first version with the "new" git show output is 1.7.7.rc1.

with git version 1.7.7.rc0:
$ git show 679e1d5cd007a0a9cb2813bd155622d7a1e904bd :
[...]
diff --git a/src/java/voldemort/store/stats/RequestCounter.java b/src/java/voldemort/store/stats/RequestCounter.java
index b012e98..c6be603 100644
--- a/src/java/voldemort/store/stats/RequestCounter.java
+++ b/src/java/voldemort/store/stats/RequestCounter.java
@@ -64,16 +64,21 @@ public class RequestCounter {
         Accumulator accum = values.get();
         long now = System.currentTimeMillis();
 
-        if(now - accum.startTimeMS > durationMS) {
-            Accumulator newWithTotal = accum.newWithTotal();
-            values.set(newWithTotal);
-
-            /*
-             * try to set.  if we fail, then someone else set it, so just keep going
-             */
-            if(values.compareAndSet(accum, newWithTotal)) {
-                return newWithTotal;
-            }
+        /*
+         *  if still in the window, just return it
+         */
+        if(now - accum.startTimeMS <= durationMS) {
+            return accum;
+        }
+
+        /*
+         * try to set.  if we fail, then someone else set it, so just return that new one
+         */
+
+        Accumulator newWithTotal = accum.newWithTotal();
+
+        if(values.compareAndSet(accum, newWithTotal)) {
+            return newWithTotal;
         }
 
         return values.get();


with git version 1.7.7.rc1:
$ git show 679e1d5cd007a0a9cb2813bd155622d7a1e904bd
[...]
diff --git a/src/java/voldemort/store/stats/RequestCounter.java b/src/java/voldemort/store/stats/RequestCounter.java
index b012e98..c6be603 100644
--- a/src/java/voldemort/store/stats/RequestCounter.java
+++ b/src/java/voldemort/store/stats/RequestCounter.java
@@ -64,16 +64,21 @@ public class RequestCounter {
         Accumulator accum = values.get();
         long now = System.currentTimeMillis();
 
-        if(now - accum.startTimeMS > durationMS) {
-            Accumulator newWithTotal = accum.newWithTotal();
-            values.set(newWithTotal);
+        /*
+         *  if still in the window, just return it
+         */
+        if(now - accum.startTimeMS <= durationMS) {
+            return accum;
+        }
 
-            /*
-             * try to set.  if we fail, then someone else set it, so just keep going
-             */
-            if(values.compareAndSet(accum, newWithTotal)) {
-                return newWithTotal;
-            }
+        /*
+         * try to set.  if we fail, then someone else set it, so just return that new one
+         */
+
+        Accumulator newWithTotal = accum.newWithTotal();
+
+        if(values.compareAndSet(accum, newWithTotal)) {
+            return newWithTotal;
         }
 
         return values.get();


The difference is, that now it's shown as two delete and two added hunks instead of one bigger delete and one bigger added hunk.

with git version 1.7.7.rc1:
$ git show 679e1d5cd007a0a9cb2813bd155622d7a1e904bd | diffstat
 RequestCounter.java |   23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

with git version 1.7.7.rc0:
$ git show 679e1d5cd007a0a9cb2813bd155622d7a1e904bd | diffstat
 RequestCounter.java |   25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

When used git version 1.7.7.rc1 I didn't observed any case where git show and git log --numstat mismatch. I'm only a little confused, that 'git show' yields different results, depending on the git version.

Greetings from Berlin
Alex

^ permalink raw reply related

* [PATCH v2 2/2] Add explanation why we do not allow to sparse checkout to empty working tree
From: Nguyễn Thái Ngọc Duy @ 2011-09-22 11:24 UTC (permalink / raw)
  To: git, Junio C Hamano
  Cc: Michael J Gruber, Joshua Jensen,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <7vk4911ux8.fsf@alter.siamese.dyndns.org>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 unpack-trees.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index fcf40a0..bacb473 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1119,8 +1119,13 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 		}
 		if (ret < 0)
 			goto return_failed;
+		/*
+		 * Sparse checkout is meant to narrow down checkout area
+		 * but it does not make sense to narrow down to empty working
+		 * tree. This is usually a mistake in sparse checkout rules.
+		 * Do not allow users to do that.
+		 */
 		if (o->result.cache_nr && empty_worktree) {
-			/* dubious---why should this fail??? */
 			ret = unpack_failed(o, "Sparse checkout leaves no entry on working directory");
 			goto done;
 		}
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH v2 1/2] sparse checkout: show error messages when worktree shaping fails
From: Nguyễn Thái Ngọc Duy @ 2011-09-22 11:24 UTC (permalink / raw)
  To: git, Junio C Hamano
  Cc: Michael J Gruber, Joshua Jensen,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <7vk4911ux8.fsf@alter.siamese.dyndns.org>

verify_* functions can queue errors up and to be printed later at
label return_failed. In case of errors, do not go to label "done"
directly because all queued messages would be dropped on the floor.

Found-by: Joshua Jensen <jjensen@workspacewhiz.com>
Tracked-down-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Now print all errors.

 t/t1011-read-tree-sparse-checkout.sh |   15 +++++++++++++++
 unpack-trees.c                       |   13 ++++++++++---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh
index 018c354..efcd8ab 100755
--- a/t/t1011-read-tree-sparse-checkout.sh
+++ b/t/t1011-read-tree-sparse-checkout.sh
@@ -234,4 +234,19 @@ test_expect_success 'read-tree --reset removes outside worktree' '
 	test_cmp empty result
 '
 
+test_expect_success 'print errors when failed to update worktree' '
+	echo sub >.git/info/sparse-checkout &&
+	git checkout -f init &&
+	mkdir sub &&
+	touch sub/added sub/addedtoo &&
+	test_must_fail git checkout top 2>actual &&
+	cat >expected <<\EOF &&
+error: The following untracked working tree files would be overwritten by checkout:
+	sub/added
+	sub/addedtoo
+Please move or remove them before you can switch branches.
+EOF
+	test_cmp expected actual
+'
+
 test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index cc616c3..fcf40a0 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1089,6 +1089,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 		 */
 		mark_new_skip_worktree(o->el, &o->result, CE_ADDED, CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);
 
+		ret = 0;
 		for (i = 0; i < o->result.cache_nr; i++) {
 			struct cache_entry *ce = o->result.cache[i];
 
@@ -1101,17 +1102,23 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 			 * correct CE_NEW_SKIP_WORKTREE
 			 */
 			if (ce->ce_flags & CE_ADDED &&
-			    verify_absent(ce, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o))
-					return -1;
+			    verify_absent(ce, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o)) {
+				if (!o->show_all_errors)
+					goto return_failed;
+				ret = -1;
+			}
 
 			if (apply_sparse_checkout(ce, o)) {
+				if (!o->show_all_errors)
+					goto return_failed;
 				ret = -1;
-				goto done;
 			}
 			if (!ce_skip_worktree(ce))
 				empty_worktree = 0;
 
 		}
+		if (ret < 0)
+			goto return_failed;
 		if (o->result.cache_nr && empty_worktree) {
 			/* dubious---why should this fail??? */
 			ret = unpack_failed(o, "Sparse checkout leaves no entry on working directory");
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* Re: What's cooking in git.git (Sep 2011, #06; Wed, 21)
From: Michael Schubert @ 2011-09-22  9:14 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: Junio C Hamano, git
In-Reply-To: <1316680641.11165.2.camel@bee.lab.cmartin.tk>

On 09/22/2011 10:37 AM, Carlos Martín Nieto wrote:
> On Wed, 2011-09-21 at 22:04 -0700, Junio C Hamano wrote:
>> * cn/eradicate-working-copy (2011-09-21) 2 commits
>>  - patch-id.c: use strbuf instead of a fixed buffer
>>  - Remove 'working copy' from the documentation and C code
> 
> It looks like that first commit sneaked in there. Shouldn't that be its
> own topic?

It's in pu twice:

On 09/22/2011 07:04 AM, Junio C Hamano wrote: 
> * cn/eradicate-working-copy (2011-09-21) 2 commits
>  - patch-id.c: use strbuf instead of a fixed buffer
>  - Remove 'working copy' from the documentation and C code

> * ms/patch-id-with-overlong-line (2011-09-21) 1 commit
>  - patch-id.c: use strbuf instead of a fixed buffer

64128da and a6c5c60

There's also a minor typo in the last sentence of the commit message.
Should I resend?

^ permalink raw reply

* Re: [PATCH] abspath: increase array size of cwd variable to PATH_MAX
From: wanghui @ 2011-09-22  8:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ramsay Jones, git
In-Reply-To: <7vty851wg9.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>
>   
>> Hmm, the subject line says "... increase array size ...", but that is not
>> necessarily what this patch is doing! :-D
>>     
>
> True; will revert it out of 'next'.
>
> Thanks for noticing.
>
>   
Hi Junio and Ramsay,

In fact, i found lots of similar usage in the git source code. E.G.

in the dir.c,
int is_inside_dir(const char *dir)
{
char cwd[PATH_MAX];
if (!dir)
return 0;
if (!getcwd(cwd, sizeof(cwd)))
die_errno("can't find the current directory");
return dir_inside_of(cwd, dir) >= 0;
}

Since this implementation is not cross-OS safe, Could we use below solution?

step1, add a new function xgetcwd() to wrapper getcwd() like this:
char *xgetcwd(void)
{
size_t size = 100;

while (1) {
char *buffer = (char *) xmalloc (size);
if (getcwd (buffer, size) == buffer)
return buffer;
free (buffer);
if (errno != ERANGE)
return 0;
size *= 2;
}
}

step2, use xgetcwd to replace all getcwd occurrence in the git source code.

This will add a little bit overhead to the git, but it is cross-OS safe.

^ permalink raw reply

* Re: What's cooking in git.git (Sep 2011, #06; Wed, 21)
From: Carlos Martín Nieto @ 2011-09-22  8:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vaa9xyxpf.fsf@alter.siamese.dyndns.org>

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

On Wed, 2011-09-21 at 22:04 -0700, Junio C Hamano wrote:
> * cn/eradicate-working-copy (2011-09-21) 2 commits
>  - patch-id.c: use strbuf instead of a fixed buffer
>  - Remove 'working copy' from the documentation and C code

It looks like that first commit sneaked in there. Shouldn't that be its
own topic?

   cmn


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: How to use git attributes to configure server-side checks?
From: Michael Haggerty @ 2011-09-22  8:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git discussion list, Jay Soffian
In-Reply-To: <7vy5xh1whq.fsf@alter.siamese.dyndns.org>

On 09/21/2011 10:17 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>> I was thinking of using git attributes to configure a server-side
>> "update" hook that does some basic sanity checking before accepting a
>> push.  I thought I could do something like
>>
>> ~/.gitattributes:
>>     *.c whitespace
>>
>> ~/crappy-vendor-code/.gitattributes:
>>     # This code doesn't conform to our standards; disable check:
>>     *.c -whitespace
>>
>> This would allow fine-grained specification of which checks are applied
>> to which files, and ensure that the hook configuration is kept
>> synchronized with changes to the content.
>
> The words "server side" automatically mean that there should be no working
> tree, and where there is no working tree there should be no index, so the
> direction should not make any difference.  The attributes that are used to
> help whitespace checks should come from project.git/info/attributes in
> such a case [*1*].

Thanks for the reply and for explaining how the index can(not) be used
for this purpose.  But what you propose is not flexible enough for me.
I would like the checking configuration to be *versioned* along with the
code.  For example, suppose my project decides to enforce a rule that
all Python code needs to be indented with spaces.  It might be that not
all of our old code adheres to this rule, and that we only want to clean
up the code in master.  I would like to be able to

1. Implement a new "check-space-indent" option in our update hook, which
is applied to any file that has the "check-space-indent" attribute.  At
first, no files have this attribute, so the new test has no effect.

2. Start cleaning up code in master.  Each time a subdirectory tree is
cleaned up, add a line like

    *.py check-space-indent

in a .gitattributes file at the root of the subdirectory tree.  While
this procedure proceeds incrementally, git ensures that code that has
been cleaned up stays cleaned up.

3. When all code is cleaned up, add the above line to the top-level
.gitattributes file in the master branch.  But if there are some parts
of the code that we don't want to clean up (for example, code acquired
from elsewhere that uses different coding standards), we can turn off
the check for that subdirectory tree.

Note that during this whole process all code passes the update hook,
because we can configure it to ignore problems in code that hasn't been
cleaned up yet.  And even at the end of the procedure, it is still
possible to commit to older branches where tabs are still used for
indentation because they don't use the new attribute.

For this to be possible, I would need to determine the git attributes to
apply to a particular file in a particular commit; something like

    git check-attr check-space-indent $SHA1:path/to/file

This does not seem to be possible today without writing my own code to
crawl and parse the gitattributes files from a particular commit.

>  (1) grab the new commits introduced to the project using rev-list, and
>      invoke "git show --check" on each and every one of them; or

Where does "git show --check" read its gitattributes (i.e.,
"whitespace") from?

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* [PATCH v3] Docs: Clarify the --tags option of `git fetch'
From: Michael Witten @ 2011-09-22  7:23 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Anatol Pomozov, Drew Northup, Andrew Ardill, Daniel Johnson, git
In-Reply-To: <7vfwjpyzds.fsf@alter.siamese.dyndns.org>

On Wed, 21 Sep 2011 21:28:15 -0700, Junio C Hamano wrote:

> I expect the readers to, and I hope the documentation to help them to,
> understand the following three basic facts and rules before diving into
> descriptions of individual options, such as the paragraph we are
> discussing:
>
>  * "git fetch" command serves two purposes:
>
>    (1) It transfers objects the repository the command is invoked in does
>    not have from the remote repository. The objects transferred are the
>    commits that are necessary to complete the ancestry chain of _some_
>    history, and data (i.e. trees and blobs) associated to use these
>    commits.
>
>    (2) It optionally can update the local refs (e.g. branches and tags)
>    with copies of the refs taken from the remote repository.
>
>  * In the above, the user needs to tell the command two things. One is
>    "where the remote repository is". The other is "what refs to fetch and
>    (optionally) how to store them". The latter "what to fetch" also
>    determines what that "_some_ history" above is (i.e. everything
>    reachable from the refs that are fetched).
>
>  * "What to fetch and how to store" have a default, recorded in the
>    repository configuration file, that is used when the user does not give
>    that information to the command from the command line. If the user does
>    give that information from the command line, that default is not used
>    at all. IOW, the command line overrides the default.
>
> With that understanding, the _only_ thing that "--tags" description needs
> to talk about is that it is an explicit way to give that "what to fetch
> and how to store" information from the command line. It instructs the
> command to fetch all the tags from the remote repository and store them
> locally.

For at least the near term, this patch may do a pretty good job of
achieving those goals without having to change too much; I do some
careful maneuvering to avoid mentioning refspecs until quite late
in the description.

8<-----------8<-----------8<-----------8<-----------8<-----------8<-----------

See the discussion starting here:

  [PATCH] Clarify that '--tags' fetches tags only
  Message-ID: <1314997486-29996-1-git-send-email-anatol.pomozov@gmail.com>
  http://thread.gmane.org/gmane.comp.version-control.git/180636

Suggested-by: Anatol Pomozov <anatol.pomozov@gmail.com>
Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 Documentation/fetch-options.txt |   31 +++++++++++++++++++++++--------
 1 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 39d326a..4cc5a80 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -56,14 +56,29 @@ endif::git-pull[]
 ifndef::git-pull[]
 -t::
 --tags::
-	Most of the tags are fetched automatically as branch
-	heads are downloaded, but tags that do not point at
-	objects reachable from the branch heads that are being
-	tracked will not be fetched by this mechanism.  This
-	flag lets all tags and their associated objects be
-	downloaded. The default behavior for a remote may be
-	specified with the remote.<name>.tagopt setting. See
-	linkgit:git-config[1].
+	Most of a remote's tags are fetched automatically as branches are
+	downloaded. However, git does not automatically fetch any tag that,
+	when 'git fetch' completes, would not be reachable from any local
+	branch head.  This option tells git to fetch all tags (and their
+	associated objects).
++
+The 'git fetch' command is often supplied with a default set of branch
+heads to fetch, but using this option tells 'git fetch' to ignore those
+defaults.
++
+This option is merely a shorthand for writing the refspec
+`refs/tags/\*:refs/tags/\*'; that is,
++
+	git fetch origin --tags
+	git fetch origin --tags frotz
++
+are equivalent to:
++
+	git fetch origin 'refs/tags/*:refs/tags/*'
+	git fetch origin frotz 'refs/tags/*:refs/tags/*'
++
+The default behavior for a remote may be specified with
+the remote.<name>.tagopt setting. See linkgit:git-config[1].
 
 --recurse-submodules[=yes|on-demand|no]::
 	This option controls if and under what conditions new commits of
-- 
1.7.6.409.ge7a85

^ permalink raw reply related

* Re: [PATCH 1/3] unpack-trees: print "Aborting" to stderr
From: Michael J Gruber @ 2011-09-22  5:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Joshua Jensen
In-Reply-To: <7v39fp1pvd.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 22.09.2011 00:40:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Forgot to update a test or two that this breaks?
> 
> In the meantime I've squashed this in.

Yikes. Sorry and thanks!

> 
>  t/t7607-merge-overwrite.sh     |    1 +
>  t/t7609-merge-co-error-msgs.sh |    5 +++++
>  2 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh
> index 72a8731..aa74184 100755
> --- a/t/t7607-merge-overwrite.sh
> +++ b/t/t7607-merge-overwrite.sh
> @@ -107,6 +107,7 @@ error: The following untracked working tree files would be overwritten by merge:
>  	sub
>  	sub2
>  Please move or remove them before you can merge.
> +Aborting
>  EOF
>  
>  test_expect_success 'will not overwrite untracked file in leading path' '
> diff --git a/t/t7609-merge-co-error-msgs.sh b/t/t7609-merge-co-error-msgs.sh
> index c994836..0e4a682 100755
> --- a/t/t7609-merge-co-error-msgs.sh
> +++ b/t/t7609-merge-co-error-msgs.sh
> @@ -32,6 +32,7 @@ error: The following untracked working tree files would be overwritten by merge:
>  	three
>  	two
>  Please move or remove them before you can merge.
> +Aborting
>  EOF
>  
>  test_expect_success 'untracked files overwritten by merge (fast and non-fast forward)' '
> @@ -56,6 +57,7 @@ Please, commit your changes or stash them before you can merge.
>  error: The following untracked working tree files would be overwritten by merge:
>  	five
>  Please move or remove them before you can merge.
> +Aborting
>  EOF
>  
>  test_expect_success 'untracked files or local changes ovewritten by merge' '
> @@ -71,6 +73,7 @@ error: Your local changes to the following files would be overwritten by checkou
>  	rep/one
>  	rep/two
>  Please, commit your changes or stash them before you can switch branches.
> +Aborting
>  EOF
>  
>  test_expect_success 'cannot switch branches because of local changes' '
> @@ -92,6 +95,7 @@ error: Your local changes to the following files would be overwritten by checkou
>  	rep/one
>  	rep/two
>  Please, commit your changes or stash them before you can switch branches.
> +Aborting
>  EOF
>  
>  test_expect_success 'not uptodate file porcelain checkout error' '
> @@ -105,6 +109,7 @@ error: Updating the following directories would lose untracked files in it:
>  	rep
>  	rep2
>  
> +Aborting
>  EOF
>  
>  test_expect_success 'not_uptodate_dir porcelain checkout error' '

^ permalink raw reply

* What's cooking in git.git (Sep 2011, #06; Wed, 21)
From: Junio C Hamano @ 2011-09-22  5:04 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'.

Here are the repositories that have my integration branches:

With maint, master, next, pu and todo:

	url = git://repo.or.cz/alt-git.git
	url = https://code.google.com/p/git-core/

With only maint and master:

	url = git://git.sourceforge.jp/gitroot/git-core/git.git
	url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core

With all the topics and integration branches:

	url = https://github.com/gitster/git

Without kernel.org working, this feature-freeze rc cycle is not really
working smoothly. I could take things slowly just like our previous
maintainer declared to do for his other project, but I probably will end
up releasing the 1.7.7 final by the end of this week without kernel.org.

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

* bw/grep-no-index-no-exclude (2011-09-15) 2 commits
 - grep --no-index: don't use git standard exclusions
 - grep: do not use --index in the short usage output

* jc/namespace-doc-with-old-asciidoc (2011-09-16) 1 commit
 - Documentation/gitnamespaces.txt: cater to older asciidoc

* jc/want-commit (2011-09-15) 1 commit
 - Allow git merge ":/<pattern>"

* jc/ls-remote-short-help (2011-09-16) 1 commit
 - ls-remote: a lone "-h" is asking for help

* jc/maint-bundle-too-quiet (2011-09-19) 1 commit
 - Teach progress eye-candy to fetch_refs_from_bundle()

* jk/filter-branch-require-clean-work-tree (2011-09-15) 1 commit
 - filter-branch: use require_clean_work_tree

* jn/gitweb-highlite-sanitise (2011-09-16) 1 commit
 - gitweb: Strip non-printable characters from syntax highlighter output

* mh/check-ref-format-3 (2011-09-16) 22 commits
 - add_ref(): verify that the refname is formatted correctly
 - resolve_ref(): expand documentation
 - resolve_ref(): also treat a too-long SHA1 as invalid
 - resolve_ref(): emit warnings for improperly-formatted references
 - resolve_ref(): verify that the input refname has the right format
 - remote: avoid passing NULL to read_ref()
 - remote: use xstrdup() instead of strdup()
 - resolve_ref(): do not follow incorrectly-formatted symbolic refs
 - resolve_ref(): extract a function get_packed_ref()
 - resolve_ref(): turn buffer into a proper string as soon as possible
 - resolve_ref(): only follow a symlink that contains a valid, normalized refname
 - resolve_ref(): use prefixcmp()
 - resolve_ref(): explicitly fail if a symlink is not readable
 - Change check_refname_format() to reject unnormalized refnames
 - Inline function refname_format_print()
 - Make collapse_slashes() allocate memory for its result
 - Do not allow ".lock" at the end of any refname component
 - Refactor check_refname_format()
 - Change check_ref_format() to take a flags argument
 - Change bad_ref_char() to return a boolean value
 - git check-ref-format: add options --allow-onelevel and --refspec-pattern
 - t1402: add some more tests

* cn/eradicate-working-copy (2011-09-21) 2 commits
 - patch-id.c: use strbuf instead of a fixed buffer
 - Remove 'working copy' from the documentation and C code

* js/bisect-no-checkout (2011-09-21) 1 commit
  (merged to 'next' on 2011-09-21 at e94ad3e)
 + bisect: fix exiting when checkout failed in bisect_start()

* mg/maint-doc-sparse-checkout (2011-09-21) 3 commits
  (merged to 'next' on 2011-09-21 at f316dec)
 + git-read-tree.txt: correct sparse-checkout and skip-worktree description
 + git-read-tree.txt: language and typography fixes
 + unpack-trees: print "Aborting" to stderr

* ms/patch-id-with-overlong-line (2011-09-21) 1 commit
 - patch-id.c: use strbuf instead of a fixed buffer

* sn/doc-update-index-assume-unchanged (2011-09-21) 1 commit
  (merged to 'next' on 2011-09-21 at 325e796)
 + Documentation/git-update-index: refer to 'ls-files'

* jc/request-pull-show-head-4 (2011-09-21) 7 commits
 - request-pull: use the branch description
 - request-pull: state what commit to expect
 - request-pull: modernize style
 - branch: teach --edit-description option
 - format-patch: use branch description in cover letter
 - branch: add read_branch_desc() helper function
 - Merge branch 'bk/ancestry-path' into jc/branch-desc
 (this branch uses bk/ancestry-path.)

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

* ph/format-patch-no-color (2011-09-12) 1 commit
  (merged to 'next' on 2011-09-12 at 20283e8)
 + format-patch: ignore ui.color

A fix for the recent regression.

--------------------------------------------------
[Stalled]

* jc/signed-push (2011-09-12) 7 commits
 - push -s: support pre-receive-signature hook
 - push -s: receiving end
 - push -s: send signed push certificate
 - push -s: skeleton
 - Split GPG interface into its own helper library
 - rename "match_refs()" to "match_push_refs()"
 - send-pack: typofix error message
 (this branch uses jc/run-receive-hook-cleanup; is tangled with jc/signed-push-3.)

This was the v2 that updated notes tree on the receiving end.

* jc/signed-push-3 (2011-09-12) 4 commits
 . push -s: signed push
 - Split GPG interface into its own helper library
 - rename "match_refs()" to "match_push_refs()"
 - send-pack: typofix error message
 (this branch uses jc/run-receive-hook-cleanup; is tangled with jc/signed-push.)

This is the third edition, that moves the preparation of the notes tree to
the sending end.

* jk/add-i-hunk-filter (2011-07-27) 5 commits
  (merged to 'next' on 2011-08-11 at 8ff9a56)
 + add--interactive: add option to autosplit hunks
 + add--interactive: allow negatation of hunk filters
 + add--interactive: allow hunk filtering on command line
 + add--interactive: factor out regex error handling
 + add--interactive: refactor patch mode argument processing

Will be dropped.

* jh/receive-count-limit (2011-05-23) 10 commits
 - receive-pack: Allow server to refuse pushes with too many objects
 - pack-objects: Estimate pack size; abort early if pack size limit is exceeded
 - send-pack/receive-pack: Allow server to refuse pushing too large packs
 - pack-objects: Allow --max-pack-size to be used together with --stdout
 - send-pack/receive-pack: Allow server to refuse pushes with too many commits
 - pack-objects: Teach new option --max-commit-count, limiting #commits in pack
 - receive-pack: Prepare for addition of the new 'limit-*' family of capabilities
 - Tighten rules for matching server capabilities in server_supports()
 - send-pack: Attempt to retrieve remote status even if pack-objects fails
 - Update technical docs to reflect side-band-64k capability in receive-pack

Would need another round to separate per-pack and per-session limits.

* jk/generation-numbers (2011-09-11) 8 commits
 - metadata-cache.c: make two functions static
 - limit "contains" traversals based on commit generation
 - check commit generation cache validity against grafts
 - pretty: support %G to show the generation number of a commit
 - commit: add commit_generation function
 - add metadata-cache infrastructure
 - decorate: allow storing values instead of pointers
 - Merge branch 'jk/tag-contains-ab' (early part) into HEAD

The initial "tag --contains" de-pessimization without need for generation
numbers is already in; backburnered.

* sr/transport-helper-fix-rfc (2011-07-19) 2 commits
 - t5800: point out that deleting branches does not work
 - t5800: document inability to push new branch with old content

Perhaps 281eee4 (revision: keep track of the end-user input from the
command line, 2011-08-25) in bk/ancestry-path would help.

* po/cygwin-backslash (2011-08-05) 2 commits
 - On Cygwin support both UNIX and DOS style path-names
 - git-compat-util: add generic find_last_dir_sep that respects is_dir_sep

Incomplete with respect to backslash processing in prefix_filename(), and
also loses the ability to escape glob specials. Perhaps drop?

--------------------------------------------------
[Cooking]

* jm/mergetool-pathspec (2011-09-16) 2 commits
 - mergetool: no longer need to save standard input
 - mergetool: Use args as pathspec to unmerged files

* nd/maint-autofix-tag-in-head (2011-09-18) 4 commits
 - Accept tags in HEAD or MERGE_HEAD
 - merge: remove global variable head[]
 - merge: use return value of resolve_ref() to determine if HEAD is invalid
 - merge: keep stash[] a local variable

* jk/maint-fetch-submodule-check-fix (2011-09-12) 1 commit
  (merged to 'next' on 2011-09-12 at 3c73b8c)
 + fetch: avoid quadratic loop checking for updated submodules
 (this branch is used by jk/argv-array.)

This probably can wait, as long as the other half of the regression fix
is in the upcoming release.

* bc/attr-ignore-case (2011-09-14) 5 commits
 - attr: read core.attributesfile from git_default_core_config
 - attr.c: respect core.ignorecase when matching attribute patterns
 - builtin/mv.c: plug miniscule memory leak
 - cleanup: use internal memory allocation wrapper functions everywhere
 - attr.c: avoid inappropriate access to strbuf "buf" member

* jc/maint-fsck-fwrite-size-check (2011-09-11) 1 commit
  (merged to 'next' on 2011-09-16 at 2258f11)
 + fsck: do not abort upon finding an empty blob

* jk/argv-array (2011-09-14) 7 commits
  (merged to 'next' on 2011-09-16 at 90feab4)
 + run_hook: use argv_array API
 + checkout: use argv_array API
 + bisect: use argv_array API
 + quote: provide sq_dequote_to_argv_array
 + refactor argv_array into generic code
 + quote.h: fix bogus comment
 + add sha1_array API docs
 (this branch uses jk/maint-fetch-submodule-check-fix.)

* js/cred-macos-x-keychain-2 (2011-09-14) 1 commit
 - contrib: add a pair of credential helpers for Mac OS X's keychain
 (this branch is tangled with jk/http-auth-keyring.)

Welcome addition to build our confidence in the jk/http-auth-keyring topic.

* rj/maint-t9159-svn-rev-notation (2011-09-21) 1 commit
 - t9159-*.sh: skip for mergeinfo test for svn <= 1.4

* tr/doc-note-rewrite (2011-09-13) 1 commit
  (merged to 'next' on 2011-09-16 at 5fe813a)
 + Documentation: basic configuration of notes.rewriteRef

Updated to a safer wording.

* jk/default-attr (2011-09-12) 1 commit
 - attr: map builtin userdiff drivers to well-known extensions

Will be re-rolled after 1.7.7 final.

* hl/iso8601-more-zone-formats (2011-09-12) 1 commit
  (merged to 'next' on 2011-09-12 at 270f5c7)
 + date.c: Support iso8601 timezone formats

* jc/run-receive-hook-cleanup (2011-09-12) 1 commit
  (merged to 'next' on 2011-09-12 at 68dd431)
 + refactor run_receive_hook()
 (this branch is used by jc/signed-push and jc/signed-push-3.)

Just to make it easier to run a hook that reads from its standard input.

* jk/for-each-ref (2011-09-08) 5 commits
  (merged to 'next' on 2011-09-14 at 36ed515)
 + for-each-ref: add split message parts to %(contents:*).
 + for-each-ref: handle multiline subjects like --pretty
 + for-each-ref: refactor subject and body placeholder parsing
 + t6300: add more body-parsing tests
 + t7004: factor out gpg setup

* wh/normalize-alt-odb-path (2011-09-07) 1 commit
  (merged to 'next' on 2011-09-14 at 96f722b)
 + sha1_file: normalize alt_odb path before comparing and storing

* fk/use-kwset-pickaxe-grep-f (2011-09-11) 2 commits
  (merged to 'next' on 2011-09-14 at 436d858)
 + obstack.c: Fix some sparse warnings
 + sparse: Fix an "Using plain integer as NULL pointer" warning

In general we would prefer to see these fixed at the upstream first, but
we have essentially forked from them at their last GPLv2 versions...

* jc/make-static (2011-09-14) 4 commits
  (merged to 'next' on 2011-09-14 at c5943ff)
 + exec_cmd.c: prepare_git_cmd() is sometimes used
 + environment.c: have_git_dir() has users on Cygwin
  (merged to 'next' on 2011-09-11 at 2acb0af)
 + vcs-svn: remove unused functions and make some static
 + make-static: master
 (this branch is tangled with jc/reflog-walk-use-only-nsha1.)

With a few fix-ups; probably needs to be ejected after 1.7.7 happens.

* rj/quietly-create-dep-dir (2011-09-11) 1 commit
  (merged to 'next' on 2011-09-12 at 93d1c6b)
 + Makefile: Make dependency directory creation less noisy

* mz/remote-rename (2011-09-11) 4 commits
 - remote: only update remote-tracking branch if updating refspec
 - remote rename: warn when refspec was not updated
 - remote: "rename o foo" should not rename ref "origin/bar"
 - remote: write correct fetch spec when renaming remote 'remote'

* cb/common-prefix-unification (2011-09-12) 3 commits
  (merged to 'next' on 2011-09-14 at 24f571f)
 + rename pathspec_prefix() to common_prefix() and move to dir.[ch]
 + consolidate pathspec_prefix and common_prefix
 + remove prefix argument from pathspec_prefix

* cb/send-email-help (2011-09-12) 1 commit
  (merged to 'next' on 2011-09-14 at ae71999)
 + send-email: add option -h

A separate set of patches to remove the hidden fully-spelled "help" from
other commands would be nice to have as companion patches as well.

* jc/fetch-pack-fsck-objects (2011-09-04) 3 commits
  (merged to 'next' on 2011-09-12 at a031347)
 + test: fetch/receive with fsckobjects
 + transfer.fsckobjects: unify fetch/receive.fsckobjects
 + fetch.fsckobjects: verify downloaded objects

We had an option to verify the sent objects before accepting a push but
lacked the corresponding option when fetching. In the light of the recent
k.org incident, a change like this would be a good addition.

* jc/fetch-verify (2011-09-01) 3 commits
  (merged to 'next' on 2011-09-12 at 3f491ab)
 + fetch: verify we have everything we need before updating our ref
 + rev-list --verify-object
 + list-objects: pass callback data to show_objects()
 (this branch uses jc/traverse-commit-list; is tangled with jc/receive-verify.)

During a fetch, we verify that the pack stream is self consistent,
but did not verify that the refs that are updated are consistent with
objects contained in the packstream, and this adds such a check.

* jc/receive-verify (2011-09-09) 6 commits
  (merged to 'next' on 2011-09-12 at 856de78)
 + receive-pack: check connectivity before concluding "git push"
 + check_everything_connected(): libify
 + check_everything_connected(): refactor to use an iterator
 + fetch: verify we have everything we need before updating our ref
 + rev-list --verify-object
 + list-objects: pass callback data to show_objects()
 (this branch uses jc/traverse-commit-list; is tangled with jc/fetch-verify.)

While accepting a push, we verify that the pack stream is self consistent,
but did not verify that the refs the push updates are consistent with
objects contained in the packstream, and this adds such a check.

* jn/maint-http-error-message (2011-09-06) 2 commits
  (merged to 'next' on 2011-09-12 at a843f03)
 + http: avoid empty error messages for some curl errors
 + http: remove extra newline in error message

* bk/ancestry-path (2011-09-15) 4 commits
  (merged to 'next' on 2011-09-15 at aa64d04)
 + t6019: avoid refname collision on case-insensitive systems
  (merged to 'next' on 2011-09-02 at d05ba5d)
 + revision: do not include sibling history in --ancestry-path output
 + revision: keep track of the end-user input from the command line
 + rev-list: Demonstrate breakage with --ancestry-path --all
 (this branch is used by jc/request-pull-show-head-4.)

* mg/branch-list (2011-09-13) 7 commits
  (merged to 'next' on 2011-09-14 at 6610a2e)
 + t3200: clean up checks for file existence
  (merged to 'next' on 2011-09-11 at 20a9cdb)
 + branch: -v does not automatically imply --list
  (merged to 'next' on 2011-09-02 at b818eae)
 + branch: allow pattern arguments
 + branch: introduce --list option
 + git-branch: introduce missing long forms for the options
 + git-tag: introduce long forms for the options
 + t6040: test branch -vv

* mm/rebase-i-exec-edit (2011-08-26) 2 commits
  (merged to 'next' on 2011-09-02 at e75b1b9)
 + rebase -i: notice and warn if "exec $cmd" modifies the index or the working tree
 + rebase -i: clean error message for --continue after failed exec

* hv/submodule-merge-search (2011-08-26) 5 commits
 - submodule: Search for merges only at end of recursive merge
 - allow multiple calls to submodule merge search for the same path
 - submodule: Demonstrate known breakage during recursive merge
 - push: Don't push a repository with unpushed submodules
  (merged to 'next' on 2011-08-24 at 398e764)
 + push: teach --recurse-submodules the on-demand option
 (this branch is tangled with fg/submodule-auto-push.)

The second from the bottom one needs to be replaced with a properly
written commit log message.

* mm/mediawiki-as-a-remote (2011-09-01) 2 commits
  (merged to 'next' on 2011-09-12 at 163c6a5)
 + git-remote-mediawiki: allow push to set MediaWiki metadata
 + Add a remote helper to interact with mediawiki (fetch & push)

Fun.

* bc/unstash-clean-crufts (2011-08-27) 4 commits
  (merged to 'next' on 2011-09-02 at 7bfd66f)
 + git-stash: remove untracked/ignored directories when stashed
 + t/t3905: add missing '&&' linkage
 + git-stash.sh: fix typo in error message
 + t/t3905: use the name 'actual' for test output, swap arguments to test_cmp

* da/make-auto-header-dependencies (2011-08-30) 1 commit
  (merged to 'next' on 2011-09-02 at e04a4af)
 + Makefile: Improve compiler header dependency check
 (this branch uses fk/make-auto-header-dependencies.)

* gb/am-hg-patch (2011-08-29) 1 commit
  (merged to 'next' on 2011-09-02 at 3edfe4c)
 + am: preliminary support for hg patches

* jc/diff-index-unpack (2011-08-29) 3 commits
  (merged to 'next' on 2011-09-02 at 4206bd9)
 + diff-index: pass pathspec down to unpack-trees machinery
 + unpack-trees: allow pruning with pathspec
 + traverse_trees(): allow pruning with pathspec

* nm/grep-object-sha1-lock (2011-08-30) 1 commit
  (merged to 'next' on 2011-09-02 at 336f57d)
 + grep: Fix race condition in delta_base_cache

* tr/mergetool-valgrind (2011-08-30) 1 commit
  (merged to 'next' on 2011-09-02 at f5f2c61)
 + Symlink mergetools scriptlets into valgrind wrappers

* fg/submodule-auto-push (2011-09-11) 2 commits
  (merged to 'next' on 2011-09-11 at 3fc86f7)
 + submodule.c: make two functions static
  (merged to 'next' on 2011-08-24 at 398e764)
 + push: teach --recurse-submodules the on-demand option
 (this branch is tangled with hv/submodule-merge-search.)

What the topic aims to achieve may make sense, but the implementation
looked somewhat suboptimal.

* jc/traverse-commit-list (2011-08-22) 3 commits
  (merged to 'next' on 2011-08-24 at df50dd7)
 + revision.c: update show_object_with_name() without using malloc()
 + revision.c: add show_object_with_name() helper function
 + rev-list: fix finish_object() call
 (this branch is used by jc/fetch-verify and jc/receive-verify.)

* fk/make-auto-header-dependencies (2011-08-18) 1 commit
  (merged to 'next' on 2011-08-24 at 3da2c25)
 + Makefile: Use computed header dependencies if the compiler supports it
 (this branch is used by da/make-auto-header-dependencies.)

* mh/iterate-refs (2011-09-11) 7 commits
 - refs.c: make create_cached_refs() static
 - Retain caches of submodule refs
 - Store the submodule name in struct cached_refs
 - Allocate cached_refs objects dynamically
 - Change the signature of read_packed_refs()
 - Access reference caches only through new function get_cached_refs()
 - Extract a function clear_cached_refs()

I did not see anything fundamentally wrong with this series, but it was
unclear what the benefit of these changes are.  If the series were to read
parts of the ref hierarchy (like refs/heads/) lazily, the story would
have been different, though.

* hv/submodule-update-none (2011-08-11) 2 commits
  (merged to 'next' on 2011-08-24 at 5302fc1)
 + add update 'none' flag to disable update of submodule by default
 + submodule: move update configuration variable further up

* jc/lookup-object-hash (2011-08-11) 6 commits
  (merged to 'next' on 2011-08-24 at 5825411)
 + object hash: replace linear probing with 4-way cuckoo hashing
 + object hash: we know the table size is a power of two
 + object hash: next_size() helper for readability
 + pack-objects --count-only
 + object.c: remove duplicated code for object hashing
 + object.c: code movement for readability

I do not think there is anything fundamentally wrong with this series, but
the risk of breakage far outweighs observed performance gain in one
particular workload. Will keep it in 'next' at least for one cycle.

* fg/submodule-git-file-git-dir (2011-08-22) 2 commits
  (merged to 'next' on 2011-08-23 at 762194e)
 + Move git-dir for submodules
 + rev-parse: add option --resolve-git-dir <path>

I do not think there is anything fundamentally wrong with this series, but
the risk of breakage outweighs any benefit for having this new
feature. Will keep it in 'next' at least for one cycle.

* jk/http-auth-keyring (2011-09-16) 21 commits
  (merged to 'next' on 2011-09-16 at b4195eb)
 + check_expirations: don't copy over same element
 + t0300: add missing EOF terminator for <<
  (merged to 'next' on 2011-09-14 at 589c7c9)
 + credential-store: use a better storage format
 + t0300: make alternate username tests more robust
 + t0300: make askpass tests a little more robust
 + credential-cache: fix expiration calculation corner cases
 + docs: minor tweaks to credentials API
  (merged to 'next' on 2011-09-11 at 491ce6a)
 + credentials: make credential_fill_gently() static
  (merged to 'next' on 2011-08-03 at b06e80e)
 + credentials: add "getpass" helper
 + credentials: add "store" helper
 + credentials: add "cache" helper
 + docs: end-user documentation for the credential subsystem
 + http: use hostname in credential description
 + allow the user to configure credential helpers
 + look for credentials in config before prompting
 + http: use credential API to get passwords
 + introduce credentials API
 + http: retry authentication failures for all http requests
 + remote-curl: don't retry auth failures with dumb protocol
 + improve httpd auth tests
 + url: decode buffers that are not NUL-terminated
 (this branch is tangled with js/cred-macos-x-keychain and js/cred-macos-x-keychain-2.)

* rr/revert-cherry-pick-continue (2011-09-11) 19 commits
  (merged to 'next' on 2011-09-11 at 7d78054)
 + builtin/revert.c: make commit_list_append() static
  (merged to 'next' on 2011-08-24 at 712c115)
 + revert: Propagate errors upwards from do_pick_commit
 + revert: Introduce --continue to continue the operation
 + revert: Don't implicitly stomp pending sequencer operation
 + revert: Remove sequencer state when no commits are pending
 + reset: Make reset remove the sequencer state
 + revert: Introduce --reset to remove sequencer state
 + revert: Make pick_commits functionally act on a commit list
 + revert: Save command-line options for continuing operation
 + revert: Save data for continuing after conflict resolution
 + revert: Don't create invalid replay_opts in parse_args
 + revert: Separate cmdline parsing from functional code
 + revert: Introduce struct to keep command-line options
 + revert: Eliminate global "commit" variable
 + revert: Rename no_replay to record_origin
 + revert: Don't check lone argument in get_encoding
 + revert: Simplify and inline add_message_to_msg
 + config: Introduce functions to write non-standard file
 + advice: Introduce error_resolve_conflict

--------------------------------------------------
[Discarded]

* mh/check-ref-format (2011-09-11) 8 commits
 . Add tools to avoid the use of unnormalized refnames.
 . Do not allow ".lock" at the end of any refname component
 . Add a library function normalize_refname()
 . Change check_ref_format() to take a flags argument
 . fixup asciidoc formatting
 . git check-ref-format: add options --allow-onelevel and --refspec-pattern
 . Change bad_ref_char() to return a boolean value
 . t1402: add some more tests

Rerolled

* jk/pager-with-alias (2011-08-19) 1 commit
 . support pager.* for aliases

* cb/maint-quiet-push (2011-09-05) 4 commits
 . t5541: avoid TAP test miscounting
 . push: old receive-pack does not understand --quiet
 . fix push --quiet via http
 . tests for push --quiet

Dropped for rerolling after 1.7.7 cycle.

* js/cred-macos-x-keychain (2011-09-11) 15 commits
  (merged to 'next' on 2011-09-12 at 8d17f94)
 + contrib: add a credential helper for Mac OS X's keychain
  (merged to 'next' on 2011-09-11 at 491ce6a)
 + credentials: make credential_fill_gently() static
  (merged to 'next' on 2011-08-03 at b06e80e)
 + credentials: add "getpass" helper
 + credentials: add "store" helper
 + credentials: add "cache" helper
 + docs: end-user documentation for the credential subsystem
 + http: use hostname in credential description
 + allow the user to configure credential helpers
 + look for credentials in config before prompting
 + http: use credential API to get passwords
 + introduce credentials API
 + http: retry authentication failures for all http requests
 + remote-curl: don't retry auth failures with dumb protocol
 + improve httpd auth tests
 + url: decode buffers that are not NUL-terminated
 (this branch is tangled with jk/http-auth-keyring and js/cred-macos-x-keychain-2.)

Reverted out of 'next'.

* jc/reflog-walk-use-only-nsha1 (2011-09-13) 4 commits
 . (baloon) teach reflog-walk to look at only new-sha1 field
 + environment.c: have_git_dir() has users on Cygwin
  (merged to 'next' on 2011-09-11 at 2acb0af)
 + vcs-svn: remove unused functions and make some static
 + make-static: master
 (this branch is tangled with jc/make-static.)

* hw/maint-abspath-cwd-limit (2011-09-21) 3 commits
  (merged to 'next' on 2011-09-21 at 210cf9a)
 + Revert 622fea4 (abspath.c: increase array size of cwd variable)
  (merged to 'next' on 2011-09-19 at 7d5e921)
 + abspath.c: increase array size of cwd variable to PATH_MAX
 + path.c: increase array size of cwd variable to PATH_MAX

Reverted out of 'next'.

* jc/request-pull-show-head (2011-09-13) 2 commits
  (merged to 'next' on 2011-09-13 at c82fb3a)
 + Revert "State what commit to expect in request-pull"
  (merged to 'next' on 2011-09-12 at c1c7b73)
 + State what commit to expect in request-pull

Reverted out of 'next'.

^ permalink raw reply

* Re: [PATCH] Clarify that '--tags' fetches tags only
From: Junio C Hamano @ 2011-09-22  4:28 UTC (permalink / raw)
  To: Michael Witten; +Cc: Andrew Ardill, Anatol Pomozov, git, computerdruid
In-Reply-To: <CAMOZ1BtPJ_Ddxo1UG2cxJMnGv9y8sR0rAyk3d_5JEz4kLsUQJQ@mail.gmail.com>

Michael Witten <mfwitten@gmail.com> writes:

> On Thu, Sep 22, 2011 at 03:13, Andrew Ardill <andrew.ardill@gmail.com> wrote:
>>
>> Maybe:
>>
>> Note that if this option is specified, then only tags
>> are fetched. No other refs, such as a remote tracking
>> branch, will be updated, even if it has been updated
>> on the remote end.
>>
>> extra info on how this option is merely a short-hand for writing the
>> refspec `refs/tags/*:refs/tags/* could go here
>
> Junio just explained why your description is inadequate and confusing.
> ...
> If I were a newbie and were to read the text that I just proferred as
> a clarification of --tags, then I would next look up just WTF a
> refspec is, and then a branch, and then...

I do agree with you that it is a futile exercise to sweep fundamental
concepts under the rug, fearing that they are too detailed and too hard
for the casual readers. By understanding how simple the fundamental
concepts and rules are, readers can have a coherent and clear mental model
of the world and synthesize these fundamental rules to understand more
complex operations the Porcelain commands offer to help every day tasks.

I however do not think, and I certainly did not mean, that the description
of "--tags" option is necessarily the place we should bombard a new user
with the term refspec and "refs/tags/*:refs/tags/*" syntax.

I expect the readers to, and I hope the documentation to help them to,
understand the following three basic facts and rules before diving into
descriptions of individual options, such as the paragraph we are
discussing:

 * "git fetch" command serves two purposes:

   (1) It transfers objects the repository the command is invoked in does
   not have from the remote repository. The objects transferred are the
   commits that are necessary to complete the ancestry chain of _some_
   history, and data (i.e. trees and blobs) associated to use these
   commits.

   (2) It optionally can update the local refs (e.g. branches and tags)
   with copies of the refs taken from the remote repository.

 * In the above, the user needs to tell the command two things. One is
   "where the remote repository is". The other is "what refs to fetch and
   (optionally) how to store them". The latter "what to fetch" also
   determines what that "_some_ history" above is (i.e. everything
   reachable from the refs that are fetched).

 * "What to fetch and how to store" have a default, recorded in the
   repository configuration file, that is used when the user does not give
   that information to the command from the command line. If the user does
   give that information from the command line, that default is not used
   at all. IOW, the command line overrides the default.

With that understanding, the _only_ thing that "--tags" description needs
to talk about is that it is an explicit way to give that "what to fetch
and how to store" information from the command line. It instructs the
command to fetch all the tags from the remote repository and store them
locally.

If the logic flow of the document presents the list of options before
helping the readers understand the above basic facts and rules, then I
think _that_ is the problem with the document we need to be addressing,
not the description of an individual option such as "--tags".

^ permalink raw reply

* [PATCH v2] Docs: Clarify the --tags option of `git fetch'
From: Michael Witten @ 2011-09-22  4:17 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Anatol Pomozov, Drew Northup, Andrew Ardill, Daniel Johnson, git
In-Reply-To: <7vsjnpz0ng.fsf@alter.siamese.dyndns.org>

On Wed, 21 Sep 2011 21:00:51 -0700, Junio C Hamano wrote:

> Michael Witten <mfwitten@gmail.com> writes:
>
>>   That is,
>>
>>     git fetch origin frotz --tags
>>
>>   is equivalent to:
>>
>>     git fetch origin frotz 'refs/tags/*:refs/tags/*'
>
> No matter what you do, please do not introduce a bad example that violates
> the usual command line syntax convention to have subcommand (e.g. fetch),
> options meant for the subcommand (e.g. --tags) and then other arguments.

Too bad; I think it really supplements the description well.

In any case:

  * The example has been changed as per your wish.
  * `short-hand' has been changed to `shorthand'.
  * Trailing whitespace has been removed from a line.

8<-----------8<-----------8<-----------8<-----------8<-----------8<-----------

See the discussion starting here:

  [PATCH] Clarify that '--tags' fetches tags only
  Message-ID: <1314997486-29996-1-git-send-email-anatol.pomozov@gmail.com>
  http://thread.gmane.org/gmane.comp.version-control.git/180636

Suggested-by: Anatol Pomozov <anatol.pomozov@gmail.com>
Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 Documentation/fetch-options.txt |   21 ++++++++++++++++++---
 1 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 39d326a..da594bd 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -61,9 +61,24 @@ ifndef::git-pull[]
 	objects reachable from the branch heads that are being
 	tracked will not be fetched by this mechanism.  This
 	flag lets all tags and their associated objects be
-	downloaded. The default behavior for a remote may be
-	specified with the remote.<name>.tagopt setting. See
-	linkgit:git-config[1].
+	downloaded.
++
+This option is merely a shorthand for writing the
+refspec `refs/tags/\*:refs/tags/\*'; consequently,
+using this option overrides any default refspec that
+would be used if no refspec were provided on the
+command line. That is,
++
+	git fetch origin --tags
+	git fetch origin --tags frotz
++
+are equivalent to:
++
+	git fetch origin 'refs/tags/*:refs/tags/*'
+	git fetch origin 'refs/tags/*:refs/tags/*' frotz
++
+The default behavior for a remote may be specified with
+the remote.<name>.tagopt setting. See linkgit:git-config[1].
 
 --recurse-submodules[=yes|on-demand|no]::
 	This option controls if and under what conditions new commits of
-- 
1.7.6.409.ge7a85

^ permalink raw reply related

* Re: [PATCH] Clarify that '--tags' fetches tags only
From: Junio C Hamano @ 2011-09-22  4:00 UTC (permalink / raw)
  To: Michael Witten; +Cc: Anatol Pomozov, git, computerdruid
In-Reply-To: <CAMOZ1Bvxc+vcofb_KyeLS7Gy=KOtX1SKv72cXA2NtwgYCWA31A@mail.gmail.com>

Michael Witten <mfwitten@gmail.com> writes:

>   That is,
>
>     git fetch origin frotz --tags
>
>   is equivalent to:
>
>     git fetch origin frotz 'refs/tags/*:refs/tags/*'

No matter what you do, please do not introduce a bad example that violates
the usual command line syntax convention to have subcommand (e.g. fetch),
options meant for the subcommand (e.g. --tags) and then other arguments.

^ permalink raw reply

* Re: [PATCH] Docs: Clarify the --tags option of `git fetch'
From: Michael Witten @ 2011-09-22  3:48 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Anatol Pomozov, Drew Northup, Andrew Ardill, Daniel Johnson, git
In-Reply-To: <58d5d7abced4468ea7587a8aeddfb5ed-mfwitten@gmail.com>

On Thu, Sep 22, 2011 at 03:39, Michael Witten <mfwitten@gmail.com> wrote:
> ...
> +This option is merely a short-hand for writing the
> ...

Junio, make that `shorthand', if you please.

^ permalink raw reply

* [PATCH] Docs: Clarify the --tags option of `git fetch'
From: Michael Witten @ 2011-09-22  3:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Anatol Pomozov, Drew Northup, Andrew Ardill, Daniel Johnson, git
In-Reply-To: <CAMOZ1BtPJ_Ddxo1UG2cxJMnGv9y8sR0rAyk3d_5JEz4kLsUQJQ@mail.gmail.com>

See the discussion starting here:

  [PATCH] Clarify that '--tags' fetches tags only
  Message-ID: <1314997486-29996-1-git-send-email-anatol.pomozov@gmail.com>
  http://thread.gmane.org/gmane.comp.version-control.git/180636

Suggested-by: Anatol Pomozov <anatol.pomozov@gmail.com>
Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 Documentation/fetch-options.txt |   21 ++++++++++++++++++---
 1 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 39d326a..fb743fa 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -61,9 +61,24 @@ ifndef::git-pull[]
 	objects reachable from the branch heads that are being
 	tracked will not be fetched by this mechanism.  This
 	flag lets all tags and their associated objects be
-	downloaded. The default behavior for a remote may be
-	specified with the remote.<name>.tagopt setting. See
-	linkgit:git-config[1].
+	downloaded.
++
+This option is merely a short-hand for writing the
+refspec `refs/tags/\*:refs/tags/\*'; consequently,
+using this option overrides any default refspec that
+would be used if no refspec were provided on the
+command line. That is,
++
+	git fetch origin --tags
+	git fetch origin frotz --tags bar
++
+are equivalent to:
++	
+	git fetch origin 'refs/tags/*:refs/tags/*'
+	git fetch origin frotz 'refs/tags/*:refs/tags/*' bar
++
+The default behavior for a remote may be specified with
+the remote.<name>.tagopt setting. See linkgit:git-config[1].
 
 --recurse-submodules[=yes|on-demand|no]::
 	This option controls if and under what conditions new commits of
-- 
1.7.6.409.ge7a85

^ permalink raw reply related

* Re: [PATCH] Clarify that '--tags' fetches tags only
From: Michael Witten @ 2011-09-22  3:24 UTC (permalink / raw)
  To: Andrew Ardill; +Cc: Junio C Hamano, Anatol Pomozov, git, computerdruid
In-Reply-To: <CAH5451nb=DTed2kAVNQmFBbGFJ9zvQAtBE+VCzKqZfGMgYpx5w@mail.gmail.com>

On Thu, Sep 22, 2011 at 03:13, Andrew Ardill <andrew.ardill@gmail.com> wrote:
> On 22 September 2011 12:07, Michael Witten <mfwitten@gmail.com> wrote:
>> On Thu, Sep 22, 2011 at 02:01, Michael Witten <mfwitten@gmail.com> wrote:
>>> On Thu, Sep 22, 2011 at 00:49, Junio C Hamano <gitster@pobox.com> wrote:
>>>> --tags is merely a short-hand for "refs/tags/*:refs/tags/*")
>>>> explicitly from the command line
>>>
>>> [Disclaimer: I don't know the code or the semantics]
>>>
>>> Why not just use that explanation?
>>>
>>>  This option is merely a short-hand for writing
>>>  the refspec `refs/tags/*:refs/tags/*'; consequently,
>>>  using this option overrides any default refspec that
>>>  would be used if no refspec were provided on the
>>>  command line. That is,
>>>
>>>    git fetch --tags origin frotz
>>>
>>>  is equivalent to:
>>>
>>>    git fetch origin frotz 'refs/tags/*:refs/tags/*'
>>>
>>> In fact, if the command line parsing performed by `git fetch'
>>> is reasonably intelligent, then it might be worthwhile
>>> to relocate `--tags' in the example:
>>>
>>>  That is,
>>>
>>>    git fetch origin frotz --tags
>>>
>>>  is equivalent to:
>>>
>>>    git fetch origin frotz 'refs/tags/*:refs/tags/*'
>>>
>>
>> Maybe this is less confusing for the example:
>>
>>  That is,
>>
>>    git fetch origin --tags
>>    git fetch origin frotz --tags bar
>>
>>  are equivalent to:
>>
>>    git fetch origin 'refs/tags/*:refs/tags/*'
>>    git fetch origin frotz 'refs/tags/*:refs/tags/*' bar
>
> This will only help people who understand that tags are just refs
> stored in refs/tags, and who understand the 'ref:ref' syntax. I think
> it is a good example to have, but people can understand the process
> and results of 'pulling/fetching a tag' without necessarily needing to
> know that tags are stored somewhere, or knowing the exact fetch
> mechanism. If these need to be documented, it should be in the
> appropriate place (which I don't think is here).
>
> I think we are skirting around the real issue, and that is that
> pulling tags will often grab objects that are *meant* to be on a
> remote branch (from the user's perspective) but that appear to be
> hanging because the remote branch ref was not updated at the same
> time. Perhaps an example or explanation of why this is the case would
> be more useful?
>
> Maybe:
>
> Note that if this option is specified, then only tags
> are fetched. No other refs, such as a remote tracking
> branch, will be updated, even if it has been updated
> on the remote end.
>
> extra info on how this option is merely a short-hand for writing the
> refspec `refs/tags/*:refs/tags/* could go here

Junio just explained why your description is inadequate and confusing.

There's so much confusion around git exactly because people are always
trying to hide just WTF is going on (especially by using TERRIBLE
terms like `branch'; see my numerous discussions).

If I were a newbie and were to read the text that I just proferred as
a clarification of --tags, then I would next look up just WTF a
refspec is, and then a branch, and then...

You see? That's exactly how it should work. People should be given
descriptions that arm them with the terms necessary to look up more
information. We need to stop writing documentation for that
hypothetical idiot who doesn't know his ass from his own face. We need
to cater to those people who intend to read documentation for the
purpose of understanding the system---not for the purpose of gettin'
shit dun with any half-baked notion that is good enough for the most
simplistic situation.

I'm sending in a patch presently.

^ permalink raw reply

* Re: [PATCH] Clarify that '--tags' fetches tags only
From: Andrew Ardill @ 2011-09-22  3:13 UTC (permalink / raw)
  To: Michael Witten; +Cc: Junio C Hamano, Anatol Pomozov, git, computerdruid
In-Reply-To: <CAMOZ1Bt6gGVd6QuRZduZ4mJ=eoZ9d7xK-WfwZ3G-+oswT0RN_Q@mail.gmail.com>

On 22 September 2011 12:07, Michael Witten <mfwitten@gmail.com> wrote:
> On Thu, Sep 22, 2011 at 02:01, Michael Witten <mfwitten@gmail.com> wrote:
>> On Thu, Sep 22, 2011 at 00:49, Junio C Hamano <gitster@pobox.com> wrote:
>>> --tags is merely a short-hand for "refs/tags/*:refs/tags/*")
>>> explicitly from the command line
>>
>> [Disclaimer: I don't know the code or the semantics]
>>
>> Why not just use that explanation?
>>
>>  This option is merely a short-hand for writing
>>  the refspec `refs/tags/*:refs/tags/*'; consequently,
>>  using this option overrides any default refspec that
>>  would be used if no refspec were provided on the
>>  command line. That is,
>>
>>    git fetch --tags origin frotz
>>
>>  is equivalent to:
>>
>>    git fetch origin frotz 'refs/tags/*:refs/tags/*'
>>
>> In fact, if the command line parsing performed by `git fetch'
>> is reasonably intelligent, then it might be worthwhile
>> to relocate `--tags' in the example:
>>
>>  That is,
>>
>>    git fetch origin frotz --tags
>>
>>  is equivalent to:
>>
>>    git fetch origin frotz 'refs/tags/*:refs/tags/*'
>>
>
> Maybe this is less confusing for the example:
>
>  That is,
>
>    git fetch origin --tags
>    git fetch origin frotz --tags bar
>
>  are equivalent to:
>
>    git fetch origin 'refs/tags/*:refs/tags/*'
>    git fetch origin frotz 'refs/tags/*:refs/tags/*' bar

This will only help people who understand that tags are just refs
stored in refs/tags, and who understand the 'ref:ref' syntax. I think
it is a good example to have, but people can understand the process
and results of 'pulling/fetching a tag' without necessarily needing to
know that tags are stored somewhere, or knowing the exact fetch
mechanism. If these need to be documented, it should be in the
appropriate place (which I don't think is here).

I think we are skirting around the real issue, and that is that
pulling tags will often grab objects that are *meant* to be on a
remote branch (from the user's perspective) but that appear to be
hanging because the remote branch ref was not updated at the same
time. Perhaps an example or explanation of why this is the case would
be more useful?

Maybe:

Note that if this option is specified, then only tags
are fetched. No other refs, such as a remote tracking
branch, will be updated, even if it has been updated
on the remote end.

extra info on how this option is merely a short-hand for writing the
refspec `refs/tags/*:refs/tags/* could go here


Regards,
Andrew

^ permalink raw reply

* [PATCH v3] Configurable hyperlinking in gitk
From: Jeff Epler @ 2011-09-22  2:15 UTC (permalink / raw)
  To: git; +Cc: Marc Branchaud, Chris Packham, Jakub Narebski, Junio C Hamano
In-Reply-To: <20110922013101.GB26880@unpythonic.net>

Many projects use project-specific notations in changelogs to refer
to bug trackers and the like.  One example is the "Closes: #12345"
notation used in Debian.

Make gitk configurable so that arbitrary strings can be turned into
clickable links that are opened in a web browser.

Signed-off-by: Jeff Epler <jepler@unpythonic.net>
---
Since the previous patch, I
 * Renamed configuration variables to get rid of the "gitk" prefix
   to encourage other git-related programs to adopt the same
   functionality.

 * Renamed configuration variables from cryptic ".re", ".sub" to less
   cryptic ".regexp" and "subst"

 * Changed the example RE to be an ERE (no \d or \M)

 * Documented that these are POSIX EREs; hopefully that's OK.  I see
   in CodingGuidelines that in git itself "a subset of BREs" are used,
   so maybe even this is too much power.  And hopefully tcl's
   re_syntax really is close enough to an ERE superset that this isn't
   a terrible lie about the initial implementation either.

 * Added a Signed-Off-By, since I've had a number of positive feedbacks
   and the only problems I've heard of (since patch v2) are the ones
   related to 'eval' in git-web--browse.

In v2 of the patch, I had fixed a problem with %-signs in URLs and
changed the documentation example.

 Documentation/config.txt |   30 +++++++++++++++++-
 gitk-git/gitk            |   75 +++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 102 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ae9913b..ffc9ccf 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1064,6 +1064,10 @@ All gitcvs variables except for 'gitcvs.usecrlfattr' and
 is one of "ext" and "pserver") to make them apply only for the given
 access method.
 
+gitk.browser::
+	Specify the browser that will be used to open links generated by
+	'linkify' configuration options.
+
 grep.lineNumber::
 	If set to true, enable '-n' option by default.
 
@@ -1317,6 +1321,28 @@ interactive.singlekey::
 	setting is silently ignored if portable keystroke input
 	is not available.
 
+linkify.<name>.regexp::
+	Specify a regular expression in the POSIX Extended Regular Expression
+	syntax defining a class of strings to automatically convert to
+	hyperlinks.  This regular expression many not span multiple lines.
+	You must also specify 'linkify.<name>.subst'.
+
+linkify.<name>.subst::
+	Specify a substitution that results in the target URL for the
+	related regular expression.  Back-references like '\1' refer
+	to capturing groups in the associated regular expression.
+	You must also specify 'linkify.<name>.regexp'.
++
+For example, to automatically link from Debian-style "Closes: #nnnn"
+message to the Debian BTS,
++
+--------
+    git config linkify.debian-bts.regexp '#([1-9][0-9]*)'
+    git config linkify.debian-bts.subst 'http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=\1'
+--------
++
+Currently, only linkgit:gitk[1] converts strings to links in this fashion.
+
 log.abbrevCommit::
 	If true, makes linkgit:git-log[1], linkgit:git-show[1], and
 	linkgit:git-whatchanged[1] assume `\--abbrev-commit`. You may
@@ -1870,5 +1896,5 @@ user.signingkey::
 
 web.browser::
 	Specify a web browser that may be used by some commands.
-	Currently only linkgit:git-instaweb[1] and linkgit:git-help[1]
-	may use it.
+	Currently only linkgit:git-instaweb[1], linkgit:gitk[1],
+	and linkgit:git-help[1] may use it.
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 4cde0c4..9db5525 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -6684,7 +6684,7 @@ proc commit_descriptor {p} {
 # append some text to the ctext widget, and make any SHA1 ID
 # that we know about be a clickable link.
 proc appendwithlinks {text tags} {
-    global ctext linknum curview
+    global ctext linknum curview linkmakers
 
     set start [$ctext index "end - 1c"]
     $ctext insert end $text $tags
@@ -6699,6 +6699,30 @@ proc appendwithlinks {text tags} {
 	setlink $linkid link$linknum
 	incr linknum
     }
+
+    if {$linkmakers == {}} return
+
+    set link_re {}
+    foreach {re rep} $linkmakers { lappend link_re $re }
+    set link_re "([join $link_re {)|(}])"
+
+    set ee 0
+    while {[regexp -indices -start $ee -- $link_re $text l]} {
+	set s [lindex $l 0]
+	set e [lindex $l 1]
+	set linktext [string range $text $s $e]
+	incr e
+	set ee $e
+
+	foreach {re rep} $linkmakers {
+	    if {![regsub $re $linktext $rep linkurl]} continue
+	    $ctext tag delete link$linknum
+	    $ctext tag add link$linknum "$start + $s c" "$start + $e c"
+	    seturllink $linkurl link$linknum
+	    incr linknum
+	    break
+	}
+    }
 }
 
 proc setlink {id lk} {
@@ -6726,6 +6750,53 @@ proc setlink {id lk} {
     }
 }
 
+proc get_link_config {} {
+    if {[catch {exec git config -z --get-regexp {^linkify\.}} linkers]} {
+	return {}
+    }
+
+    set linktypes [list]
+    foreach item [split $linkers "\0"] {
+	if {$item == ""} continue
+	if {![regexp {linkify\.(\S+)\.(regexp|subst)\s(.*)} $item _ k t v]} {
+	    continue
+	}
+	set linkconfig($t,$k) $v
+	if {$t == "regexp"} { lappend linktypes $k }
+    }
+
+    set linkmakers [list]
+    foreach k $linktypes {
+	if {![info exists linkconfig(subst,$k)]} {
+	    puts stderr "Warning: link `$k' is missing a substitution string"
+	} elseif {[catch {regexp -inline -- $linkconfig(regexp,$k) ""} err]} {
+	    puts stderr "Warning: link `$k': $err"
+	} else {
+	    lappend linkmakers $linkconfig(regexp,$k) $linkconfig(subst,$k)
+	}
+	unset linkconfig(regexp,$k)
+	unset -nocomplain linkconfig(subst,$k)
+    }
+    foreach k [array names linkconfig] {
+	regexp "subst,(.*)" $k _ k
+	puts stderr "Warning: link `$k' is missing a regular expression"
+    }
+    set linkmakers
+}
+
+proc openlink {url} {
+    exec git web--browse --config=gitk.browser $url &
+}
+
+proc seturllink {url lk} {
+    set qurl [string map {% %%} $url]
+    global ctext
+    $ctext tag conf $lk -foreground blue -underline 1
+    $ctext tag bind $lk <1> [list openlink $qurl]
+    $ctext tag bind $lk <Enter> {linkcursor %W 1}
+    $ctext tag bind $lk <Leave> {linkcursor %W -1}
+}
+
 proc appendshortlink {id {pre {}} {post {}}} {
     global ctext linknum
 
@@ -11693,6 +11764,8 @@ if {[tk windowingsystem] eq "win32"} {
     focus -force .
 }
 
+set linkmakers [get_link_config]
+
 getcommits {}
 
 # Local variables:
-- 
1.7.2.5

^ permalink raw reply related

* Re: [PATCH] abspath: increase array size of cwd variable to PATH_MAX
From: wanghui @ 2011-09-22  2:09 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Junio C Hamano, git
In-Reply-To: <4E791A40.6040102@ramsay1.demon.co.uk>

Ramsay Jones wrote:
> Junio C Hamano wrote:
>   
>> Wang Hui <Hui.Wang@windriver.com> writes:
>>
>>     
>>> diff --git a/abspath.c b/abspath.c
>>> index f04ac18..2ce1db9 100644
>>> --- a/abspath.c
>>> +++ b/abspath.c
>>> @@ -24,7 +24,7 @@ int is_directory(const char *path)
>>>  const char *real_path(const char *path)
>>>  {
>>>  	static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
>>> -	char cwd[1024] = "";
>>> +	char cwd[PATH_MAX] = "";
>>>       
>> Thanks.
>>
>> This does not make things worse but in the longer term we should move away
>> from using PATH_MAX in general.
>>     
>
> Hmm, the subject line says "... increase array size ...", but that is not
> necessarily what this patch is doing! :-D
>
> Yes, on some platforms PATH_MAX will be larger than 1024 (e.g. 4096 on Linux),
> but that is not even true of all POSIX systems. POSIX defines the *minimum*
> value of PATH_MAX that systems must support (as #define _POSIX_PATH_MAX) of 255.
> [it also requires that POSIX conforming applications must not *require* a value
> larger than 255].
>
> However, we don't have to look too far to find systems with much smaller values.
> On Cygwin, for example:
>
>     $ cat -n junk.c
>          1  #include <stdio.h>
>          2  #include <limits.h>
>          3
>          4  int main(int argc, char *argv[])
>          5  {
>          6          printf("PATH_MAX is %d\n", PATH_MAX);
>          7          return 0;
>          8  }
>     $ gcc -o junk junk.c
>     $ ./junk
>     $ PATH_MAX is 260
>     $ 
>
> On MinGW the answer is 259.
>
> So, I certainly agree that moving away from PATH_MAX is a good idea, but I'm
> not sure I agree that this patch "does not make things worse" ... (I haven't
> given it *any* thought!).
>   
Hi Ramsay,

Do you mean the PATH_MAX of a system should not be the limitation for 
the git. That is to say, the git can handle the path which has name 
longer than PATH_MAX? If it is, my patch is not needed here. :-)
> [Also, note commits f66cf96, fd55a19, 620e2bb, etc...]
>
> ATB,
> Ramsay Jones
>
>
>   

^ permalink raw reply

* Re: [PATCH] Clarify that '--tags' fetches tags only
From: Michael Witten @ 2011-09-22  2:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Anatol Pomozov, git, computerdruid
In-Reply-To: <CAMOZ1Bvxc+vcofb_KyeLS7Gy=KOtX1SKv72cXA2NtwgYCWA31A@mail.gmail.com>

On Thu, Sep 22, 2011 at 02:01, Michael Witten <mfwitten@gmail.com> wrote:
> On Thu, Sep 22, 2011 at 00:49, Junio C Hamano <gitster@pobox.com> wrote:
>> --tags is merely a short-hand for "refs/tags/*:refs/tags/*")
>> explicitly from the command line
>
> [Disclaimer: I don't know the code or the semantics]
>
> Why not just use that explanation?
>
>  This option is merely a short-hand for writing
>  the refspec `refs/tags/*:refs/tags/*'; consequently,
>  using this option overrides any default refspec that
>  would be used if no refspec were provided on the
>  command line. That is,
>
>    git fetch --tags origin frotz
>
>  is equivalent to:
>
>    git fetch origin frotz 'refs/tags/*:refs/tags/*'
>
> In fact, if the command line parsing performed by `git fetch'
> is reasonably intelligent, then it might be worthwhile
> to relocate `--tags' in the example:
>
>  That is,
>
>    git fetch origin frotz --tags
>
>  is equivalent to:
>
>    git fetch origin frotz 'refs/tags/*:refs/tags/*'
>

Maybe this is less confusing for the example:

  That is,

    git fetch origin --tags
    git fetch origin frotz --tags bar

  are equivalent to:

    git fetch origin 'refs/tags/*:refs/tags/*'
    git fetch origin frotz 'refs/tags/*:refs/tags/*' bar

^ permalink raw reply

* Re: [PATCH] Clarify that '--tags' fetches tags only
From: Michael Witten @ 2011-09-22  2:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Anatol Pomozov, git, computerdruid
In-Reply-To: <7vwrd1z9it.fsf@alter.siamese.dyndns.org>

On Thu, Sep 22, 2011 at 00:49, Junio C Hamano <gitster@pobox.com> wrote:
> --tags is merely a short-hand for "refs/tags/*:refs/tags/*")
> explicitly from the command line

[Disclaimer: I don't know the code or the semantics]

Why not just use that explanation?

  This option is merely a short-hand for writing
  the refspec `refs/tags/*:refs/tags/*'; consequently,
  using this option overrides any default refspec that
  would be used if no refspec were provided on the
  command line. That is,

    git fetch --tags origin frotz

  is equivalent to:

    git fetch origin frotz 'refs/tags/*:refs/tags/*'

In fact, if the command line parsing performed by `git fetch'
is reasonably intelligent, then it might be worthwhile
to relocate `--tags' in the example:

  That is,

    git fetch origin frotz --tags

  is equivalent to:

    git fetch origin frotz 'refs/tags/*:refs/tags/*'

^ 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