Git development
 help / color / mirror / Atom feed
* Re: [PATCH 6/6] send-email: do not prompt for explicit repo ident
From: Jonathan Nieder @ 2012-11-14 17:18 UTC (permalink / raw)
  To: Jeff King; +Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano
In-Reply-To: <20121113165327.GF12626@sigill.intra.peff.net>

Jeff King wrote:

> If git-send-email is configured with sendemail.from, we will
> not prompt the user for the "From" address of the emails.
> If it is not configured, we prompt the user, but provide the
> repo author or committer as a default.  Even though we
> probably have a sensible value for the default, the prompt
> is a safety check in case git generated an incorrect
> implicit ident string.

I haven't read the code carefully, but this behavior sounds sensible,
so for what it's worth,
Acked-by: Jonathan Nieder <jrnieder@gmail.com>

[...]
> The test scripts need to be adjusted to not expect a prompt
> for the sender, since they always have the author explicitly
> defined in the environment. Unfortunately, we cannot
> reliably test that prompting still happens in the implicit
> case, as send-email will produce inconsistent results
> depending on the machine config (if we cannot find a FQDN,
> "git var" will barf, causing us to exit early;

At first this sounded like a bug to me --- how could the user keep
working without the sysadmin intervening?

But then I remembered that the user can set her name and email in
.gitconfig and probably would want to in such a setup anyway.

When someone writes such a test, I think it could check that git
either prompts or writes a message advising to configure the user
email, no?  Waiting until later for that seems fine to me, though.

Thanks,
Jonathan

^ permalink raw reply

* RE: push branch descriptions
From: Pyeron, Jason J CTR (US) @ 2012-11-14 17:21 UTC (permalink / raw)
  To: git
In-Reply-To: <CAB9Jk9BN6vTnpPa8rv1Yumn2Os=XU=tEn3=j2QJJK1GpajY=VQ@mail.gmail.com>

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

> -----Original Message-----
> From: Angelo Borsotti
> Sent: Wednesday, November 14, 2012 9:51 AM
> 
> Hi Junio,
> 
> > It would conceptually be a lot cleaner to treat updating of remote
> > Ibranch description as a separate "repository management" class of
> > Ioperation, similar to setting the repository description stored in
> > I$GIT_DIR/description.
> 
> I agree, it should be a distinct operation. I was thinking that when
> you have a remote bare repository, the normal way of adding contents
> to it is to push to it, and thus also adding a description should be
> done with some sort of pushing. Creating branches is also normally
> done with a push (think how difficult it is to create a branch in a
> bare repository when the HEAD is not set ...).

Only if the push were to create the branch...


> On 14 November 2012 14:57, Junio C Hamano <gitster@pobox.com> wrote:
> > Angelo Borsotti <angelo.borsotti@gmail.com> writes:
> >
> >> currently, there is no means to push a branch description to a
> remote
> >> repository. It is possible to create a branch, but not to set its
> >> description.
> >
> > Correct.  You have to go to the remote repository and run "git
> > branch --edit-description" there; there is currently no way to do
> > this remotely, which may be an issue, but...
> >
> >> Would not be more correct to push also branch descriptions when
> >> branches are pushed?
> >
> > ... I do not think "git push" is the best place to do so, given the
> > inherently local nature of branches and branch descriptions.
> >
> > Imagine the project creates a branch "magic" to enhance its system
> > with magic words.  The description for the "magic" branch in the
> > project may say "support magic words" or something.
> >
> > You and your friend are tasked to add a handful of magic words,
> > e.g. "xyzzy", "frotz" and "nitfol".  You may start your work like so
> > on your "magic-xyzzy" branch:
> >
> >     $ git clone git://example.com/zork.git/
> >     $ git checkout -b magic-xyzzy -t origin/magic

And here the branch description should copy from origin/magic.

> >
> > And you say something like "add xyzzy magic" in its branch
> > description.
> >
> >     $ git branch --edit-description magic-xyzzy
> >

And now it is edited

> > After finishing your work, you may push it
> >
> >     $ git push origin magic-xyzzy:magic
> >
> > Should the description of the subtask "add xyzzy magic" overwrite
> > the purpose of the project wide "magic" branch "support magic words"?
> > Most likely not.

Never overwrite anything.

> >
> > The local nature of the description becomes even more clear if you
> > imagine the case where the push at the last stage gets rejected due
> > to non-fast-forward error (in other words, your friend has already
> > pushed her support of the "frotz" magic to the "magic" branch.
> >
> > In fact, you would normally not directly push your magic-xyzzy
> > branch to the magic branch, but you would do something like this
> > once you are done:
> >
> >     $ git checkout -b magic -t origin/magic
> >     $ git pull origin ;# to update with her work
> >     $ git merge magic-xyzzy
> >     $ git push origin magic
> >
> > And the last "merge" is where the description for your magic-xyzzy
> > is used to fill the commit log template for you to explain your
> > merge (that is, you are merging a branch whose description is "add
> > xyzzy magic").  There is no reason to propagate the description of
> > your magic-xyzzy topic to the description of shared magic branch
> > when you push, as this merge commit already records what the branch
> > that was merged was about.
> >
> > So you could modify "git push" to set the branch description when
> > you push to create a branch remotely, but in general, "git push"
> > should not be updating the branch description with the description
> > of your local branch.  This comes as a consequence of the fact that
> > the purpose of the branch in the remote central repository is, more
> > often than not, different from the purpose of the corresponding
> > branch in your repository.
> >

But pulling such into a new branch should copy the description (unless explicitly set)

> > It would conceptually be a lot cleaner to treat updating of remote
> > branch description as a separate "repository management" class of
> > operation, similar to setting the repository description stored in
> > $GIT_DIR/description.


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5615 bytes --]

^ permalink raw reply

* Re: [PATCH v3 0/5] push: update remote tags only with force
From: Junio C Hamano @ 2012-11-14 17:32 UTC (permalink / raw)
  To: Angelo Borsotti
  Cc: Chris Rorvick, git, Drew Northup, Michael Haggerty, Philip Oakley,
	Johannes Sixt, Kacper Kornet, Jeff King, Felipe Contreras
In-Reply-To: <CAB9Jk9DAwaLw2bTqj5x_zxRcFqn7s=nmGi=Jc_SD38vFoszBZg@mail.gmail.com>

Angelo Borsotti <angelo.borsotti@gmail.com> writes:

> actually, I proposed to add a key in config files, e.g.
> pushTagsNoChange to be set in the remote repo do disallow changes to
> tags, similar to pushNonFastForward that disallows non-fastforward
> changes to branches. I still have the impression that this is simple
> and clear, and allows the owner of the remote repository to enforce
> the policy s/he wants on her/his repository.

That is an independent issue of deciding to accept or reject
receiving a push from outside, no?  You can implement any such
policy in the pre-receive hook on the receiving end with a simple
and clear manner, instead of adding specific logic to enforce a
single hardcoded policy to the code that is flipped on with a
configuration variable.

In any case, I thought this series was about users who run "push"
voluntarily stopping themselves from pushing updates to tags that
may happen to fast-forward, so if we were to go with the
configuration route, the suggestion would be more like

    [push]
	updateNeedsForce = refs/tags/:refs/frotz/

or perhaps

    [remote "origin"]
	updateNeedsForce = refs/tags/:refs/frotz/

if we want to configure it per-remote, to specify that you would
need to say "--force" to update the refs in the listed hierarchies.

Then your patch series could become just the matter of declaring
that the value of push.updateNeedsForce, when unspecified, defaults
to "refs/tags/".

^ permalink raw reply

* Re: [PATCHv3 3/4] git-status: show short sequencer state
From: Junio C Hamano @ 2012-11-14 17:44 UTC (permalink / raw)
  To: Phil Hord
  Cc: git, phil.hord, Jeff King, konglu, Matthieu Moy, Kong Lucien,
	Duperray Valentin, Jonas Franck, Nguy Thomas
In-Reply-To: <50A3A040.7040304@cisco.com>

Phil Hord <hordp@cisco.com> writes:

> Consider the usage:
>
>   git status   # show work-tree status
>   git status --short  # show short work-tree status
>   git status --tokens  # show work-tree status in token form

OK, your --tokens is more about *how* things are output, but it is
unclear how it would interact with --short.  I had an impression
that you are basing your output on the short output, whose existing
record include "##" (that shows the branch names and states), and
"MM", "A " and friends (that show the per-file states), by adding
new record types that shows tree-wide states.

> But maybe "--tokens" has some better meaning that someone will want to
> use in the future.  I'm not married to it.  But "git status" already
> means "Show the working tree status".  So "git status --show-tree-state"
> sounds redundant or meaningless.

I didn't mean to say that you have to spell out all these words;
"show" and "state" are redundant.

The important part is that unlike the existing "per-file" state the
"status" command is showing, the option is to add "tree-wide" state
to the output, and my suggestion was to pick a word that makes it
clear, rather than using "output is done using tokens" without
saying "what is being output in tokenized form".

^ permalink raw reply

* Re: push branch descriptions
From: Junio C Hamano @ 2012-11-14 17:58 UTC (permalink / raw)
  To: Pyeron, Jason J CTR (US); +Cc: git
In-Reply-To: <871B6C10EBEFE342A772D1159D13208537AABCCA@umechphj.easf.csd.disa.mil>

"Pyeron, Jason J CTR (US)" <jason.j.pyeron.ctr@mail.mil> writes:

>> > Imagine the project creates a branch "magic" to enhance its system
>> > with magic words.  The description for the "magic" branch in the
>> > project may say "support magic words" or something.
>> >
>> > You and your friend are tasked to add a handful of magic words,
>> > e.g. "xyzzy", "frotz" and "nitfol".  You may start your work like so
>> > on your "magic-xyzzy" branch:
>> >
>> >     $ git clone git://example.com/zork.git/
>> >     $ git checkout -b magic-xyzzy -t origin/magic
>
> And here the branch description should copy from origin/magic.

I doubt it should.  The purpose of the "magic" branch at the remote
in my example were to "support magic words" (without limiting which
magic words the project wants to support) and that is what the
description over there may say, while the purpose of the local
"magic-xyzzy" branch you create in order to add the support for
"xyzzy" magic is just one small subtask of it.

That is what I meant by "the inherently local nature of the branches
and branch descriptions".  Git as a distributed system works well
exactly because what each repository has is inherently local, and
people can do whatever they want in their own repositories, while
allowing collaboration among participants by pulling and pushing
histories that share compatible (note: not necessarily "identical")
goals.  "support magic words" being a superset of "add xyzzy magic"
is an example of this principle.  They have different goals (and
that is why propagating the description of your "magic-xyzzy" to the
project global "magic" is a wrong thing to do), but from the point
of view of the project global "magic" branch, what your "magic-xyzzy"
branch wanted to do is compatible with its larger goal (and that is
why merging to "magic" from "magic-xyzzy" is a good thing, while
merging the other way is frowned upon in general).

^ permalink raw reply

* Re: [regression] Newer gits cannot clone any remote repos
From: Douglas Mencken @ 2012-11-14 18:01 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Torsten Bögershausen, Ramsay Jones, git
In-Reply-To: <mvmvcd8f8ml.fsf@g208.suse.de>

>>> I cannot reproduce the problem (on openSUSE 12.2).
>>
>> You do need multiple CPU/multi-core machine, as I got it.
>
> Which is what I have.

Then try to build *vanilla* git 1.8.0, not OpenSuSE's one (with a lot
of patches inside srcrpm).

^ permalink raw reply

* v1.8.0-150-gb0b00a3: Weird documentation for git-rm
From: Horst H. von Brand @ 2012-11-14 18:15 UTC (permalink / raw)
  To: git; +Cc: gitster, Horst H. von Brand

The file Documentation/git-rm.txt says:

---8><---- 8><-------
Using ``git add -A''
~~~~~~~~~~~~~~~~~~~~
When accepting a new code drop for a vendor branch, you probably
want to record both the removal of paths and additions of new paths
as well as modifications of existing paths.

Typically you would first remove all tracked files from the working
tree using this command:

Submodules
~~~~~~~~~~
---8><---- 8><-------

I believe a command is missing here.

The explanation also leaves me scratching my head...

(In Fedora's git-1.7.7.6-1.fc16.x86_64 there is a command and some further
explanation).
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                    Fono: +56 32 2654431
Universidad Tecnica Federico Santa Maria             +56 32 2654239
Casilla 110-V, Valparaiso, Chile 2340000       Fax:  +56 32 2797513

^ permalink raw reply

* [PATCH] docs: move submodule section
From: Matt Kraai @ 2012-11-14 18:49 UTC (permalink / raw)
  To: git, gitster, Horst H. von Brand; +Cc: Matt Kraai
In-Reply-To: <201211141815.qAEIFTBi017644@netbook1.inf.utfsm.cl>

293ab15e ("submodule: teach rm to remove submodules unless they
contain a git directory", 2012-09-26) inserted the "Submodules"
section between a sentence describing a command and the command.  Move
the "Submodules" section further down.

Signed-off-by: Matt Kraai <kraai@ftbfs.org>
---
 Documentation/git-rm.txt |   30 +++++++++++++++---------------
 1 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 882cb11..262436b 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -107,21 +107,6 @@ as well as modifications of existing paths.
 Typically you would first remove all tracked files from the working
 tree using this command:
 
-Submodules
-~~~~~~~~~~
-Only submodules using a gitfile (which means they were cloned
-with a git version 1.7.8 or newer) will be removed from the work
-tree, as their repository lives inside the .git directory of the
-superproject. If a submodule (or one of those nested inside it)
-still uses a .git directory, `git rm` will fail - no matter if forced
-or not - to protect the submodule's history.
-
-A submodule is considered up-to-date when the HEAD is the same as
-recorded in the index, no tracked files are modified and no untracked
-files that aren't ignored are present in the submodules work tree.
-Ignored files are deemed expendable and won't stop a submodule's work
-tree from being removed.
-
 ----------------
 git ls-files -z | xargs -0 rm -f
 ----------------
@@ -149,6 +134,21 @@ use the following command:
 git diff --name-only --diff-filter=D -z | xargs -0 git rm --cached
 ----------------
 
+Submodules
+~~~~~~~~~~
+Only submodules using a gitfile (which means they were cloned
+with a git version 1.7.8 or newer) will be removed from the work
+tree, as their repository lives inside the .git directory of the
+superproject. If a submodule (or one of those nested inside it)
+still uses a .git directory, `git rm` will fail - no matter if forced
+or not - to protect the submodule's history.
+
+A submodule is considered up-to-date when the HEAD is the same as
+recorded in the index, no tracked files are modified and no untracked
+files that aren't ignored are present in the submodules work tree.
+Ignored files are deemed expendable and won't stop a submodule's work
+tree from being removed.
+
 EXAMPLES
 --------
 `git rm Documentation/\*.txt`::
-- 
1.7.2.5

^ permalink raw reply related

* Re: [PATCH] docs: move submodule section
From: Matt Kraai @ 2012-11-14 18:50 UTC (permalink / raw)
  To: git, gitster, Horst H. von Brand
In-Reply-To: <1352918944-26606-1-git-send-email-kraai@ftbfs.org>

On Wed, Nov 14, 2012 at 10:49:04AM -0800, Matt Kraai wrote:
> 293ab15e ("submodule: teach rm to remove submodules unless they
> contain a git directory", 2012-09-26) inserted the "Submodules"
> section between a sentence describing a command and the command.  Move
> the "Submodules" section further down.
> 
> Signed-off-by: Matt Kraai <kraai@ftbfs.org>

Sorry, I amended the patch to include a Reported-by line but forgot to
reformat it before sending.  It should have included:

 Reported-by: "Horst H. von Brand" <vonbrand@inf.utfsm.cl>

-- 
Matt Kraai
https://ftbfs.org/kraai

^ permalink raw reply

* Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)
From: Jeff King @ 2012-11-14 19:02 UTC (permalink / raw)
  To: Mark Levedahl; +Cc: Torsten Bögershausen, git
In-Reply-To: <50A2F17D.4010907@gmail.com>

On Tue, Nov 13, 2012 at 08:18:53PM -0500, Mark Levedahl wrote:

> On 11/13/2012 03:45 PM, Torsten Bögershausen wrote:
> >>* ml/cygwin-mingw-headers (2012-11-12) 1 commit
> >>  - Update cygwin.c for new mingw-64 win32 api headers
> >>
> >>  Make git work on newer cygwin.
> >>
> >>  Will merge to 'next'.
> >(Sorry for late answer, I managed to test the original patch minutes before Peff merged it to pu)
> >(And thanks for maintaining git)
> >
> >Is everybody using cygwin happy with this?
> >
> >I managed to compile on a fresh installed cygwin,
> >but failed to compile under 1.7.7, see below.
> >Is there a way we can achieve to compile git both under "old" and "new" cygwin 1.7 ?
> >Or is this not worth the effort?
> >
> I found no version info defined that could be used to automatically
> switch between the old and current headers. You can always
> 
>     make V15_MINGW_HEADERS=1 ...
> 
> to force using the old set if you do not wish to update your installation.

Should we keep the code change, then, but not flip the default (i.e.,
make people on the newer version opt into it)? I am not clear on how
common the newer include system is. Of course, auto-detecting would be
the ideal.

-Peff

^ permalink raw reply

* Re: [PATCH 1/6] ident: make user_ident_explicitly_given private
From: Jeff King @ 2012-11-14 19:11 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano,
	Santi Béjar
In-Reply-To: <20121114164457.GA6858@elie.Belkin>

On Wed, Nov 14, 2012 at 08:44:57AM -0800, Jonathan Nieder wrote:

> > -#define IDENT_NAME_GIVEN 01
> > -#define IDENT_MAIL_GIVEN 02
> > -#define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
> > -extern int user_ident_explicitly_given;
> >  extern int user_ident_sufficiently_given(void);
> 
> In v1.5.6-rc0~56^2 (2008-05-04) "user_ident_explicitly_given" was
> introduced as a global for communication between config, ident, and
> builtin-commit.  In v1.7.0-rc0~72^2 (2010-01-07) readers switched to
> using the common wrapper user_ident_sufficiently_given().  After
> v1.7.11-rc1~15^2~18 (2012-05-21) the var is only written in ident.c,
> and the variable can finally be made static.

Thanks for digging up the history. I remembered trying to do this before
during the ident refactoring that went into v1.7.11, but it didn't work
then for some reason (I think it was probably an earlier iteration of
the series, and then I forgot to revisit it during the final one).

I'll add your explanation to the commit message if I re-roll.

-Peff

^ permalink raw reply

* Re: [PATCHv3 3/4] git-status: show short sequencer state
From: Phil Hord @ 2012-11-14 19:14 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, phil.hord, Jeff King, konglu, Matthieu Moy, Kong Lucien,
	Duperray Valentin, Jonas Franck, Nguy Thomas
In-Reply-To: <7vfw4caxkh.fsf@alter.siamese.dyndns.org>


Junio C Hamano wrote:
> Phil Hord <hordp@cisco.com> writes:
>
>> Consider the usage:
>>
>>   git status   # show work-tree status
>>   git status --short  # show short work-tree status
>>   git status --tokens  # show work-tree status in token form
> OK, your --tokens is more about *how* things are output, but it is
> unclear how it would interact with --short.  I had an impression
> that you are basing your output on the short output, whose existing
> record include "##" (that shows the branch names and states), and
> "MM", "A " and friends (that show the per-file states), by adding
> new record types that shows tree-wide states.

I am, but I don't much care for the "##" prefix, especially when
combined with --null, for example.  I'm inclined to remove it when
--short is not provided, specifically to give scripts an easier time of
parsing.  But scripts are likely to need "--porcelain" as well, and
currently that implies "--short".  But I suppose another combination
could be meaningful.

  # tokens only
  $ git status --tree
  changed-files

  # tokens and short-status
  $ git status --tree --short  
  ## changed-files
   M foo.txt

  # short-status only
  $ git status --porcelain
   M foo.txt

  # tokens only?
  $ git status --tree --porcelain
  changed-files

I think this spaghettify's the ui too much.  Maybe this instead:

  # undecorated tokens only
  $ git status --tree=porcelain
  changed-files


>
>> But maybe "--tokens" has some better meaning that someone will want to
>> use in the future.  I'm not married to it.  But "git status" already
>> means "Show the working tree status".  So "git status --show-tree-state"
>> sounds redundant or meaningless.
> I didn't mean to say that you have to spell out all these words;
> "show" and "state" are redundant.
>
> The important part is that unlike the existing "per-file" state the
> "status" command is showing, the option is to add "tree-wide" state
> to the output, and my suggestion was to pick a word that makes it
> clear, rather than using "output is done using tokens" without
> saying "what is being output in tokenized form".

Thanks for clarifying. 

Phil

^ permalink raw reply

* Re: [PATCH 3/6] var: accept multiple variables on the command line
From: Jeff King @ 2012-11-14 19:26 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano
In-Reply-To: <20121114170148.GB6858@elie.Belkin>

On Wed, Nov 14, 2012 at 09:01:48AM -0800, Jonathan Nieder wrote:

> >  DESCRIPTION
> >  -----------
> > -Prints a git logical variable.
> > +Prints one or more git logical variables, separated by newlines.
> > +
> > +Note that some variables may contain newlines themselves
> 
> Maybe a -z option to NUL-terminate values would be useful some day.

Yeah, I thought about that but stopped short. The intended caller in my
series is Git.pm, whose command() splits on newlines. Although it is
perl...I suspect doing:

  local $/ = "\0";
  my @entries = command(...);

would work. For ident variables, we know they don't contain a newline,
though.

> > --- a/builtin/var.c
> > +++ b/builtin/var.c
> > @@ -73,8 +73,7 @@ static int show_config(const char *var, const char *value, void *cb)
> >  
> >  int cmd_var(int argc, const char **argv, const char *prefix)
> >  {
> > -	const char *val = NULL;
> > -	if (argc != 2)
> > +	if (argc < 2)
> >  		usage(var_usage);
> >  
> >  	if (strcmp(argv[1], "-l") == 0) {
> 
> What should happen if I pass "-l" followed by other arguments?

Good catch. Probably we should just call usage() once we see "-l"
and (argc > 2), which matches the previous behavior. I don't see much
point in listing specific variables after having listed them all.

I was also tempted to convert to parse_options, but I don't think that
really buys us anything (we could detect the option in "git var foo -l
bar", but since we are not going to do anything useful in such a case,
there is not much point).

> > +	test_tick &&
> > +	echo "A U Thor <author@example.com> 1112911993 -0700" >expect &&
> 
> Do we need to hardcode the timestamp?  Something like
> 
> 	test_cmp_filtered () {
> 		expect=$1 actual=$2 &&
> 		sed -e 's/[0-9][0-9]* [-+][0-9][0-9][0-9][0-9]/TIMESTAMP" \
> 			<"$actual" >"$actual.filtered" &&
> 		test_cmp "$expect" "$actual.filtered"
> 	}

No, we don't have to. I was just hoping to keep the tests simple by not
doing any parsing trickery. The test_tick keeps it stable, but as you
note, it is not robust to reordering. I think it would be sufficient to
just put $GIT_COMMITTER_DATE into the expected output.

I'll fix both in a re-roll.

Thanks.

-Peff

^ permalink raw reply

* Re: [PATCH nd/wildmatch] Correct Git's version of isprint and isspace
From: René Scharfe @ 2012-11-14 19:30 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Nguyễn Thái Ngọc Duy, Git Mailing List,
	Junio C Hamano, schnhrr
In-Reply-To: <CA+55aFynRG-CbSp-aLoo1iZTvfBWMgt6kwrPiQjSZJ0ZzraDKQ@mail.gmail.com>

Am 13.11.2012 20:50, schrieb Linus Torvalds:
> On Tue, Nov 13, 2012 at 11:40 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> I have to wonder why you care? As far as I'm concerned, the only valid
>> space is space, TAB and CR/LF.
>>
>> Anything else is *noise*, not space. What's the reason for even caring?
>
> Btw, expanding the whitespace selection may actually be very
> counter-productive. It is used primarily for things like removing
> extraneous space at the end of lines etc, and for that, the current
> selection of SPACE, TAB and LF/CR is the right thing to do.
>
> Adding things like FF etc - that are *technically* whitespace, but
> aren't the normal kind of silent whitespace - is potentially going to
> change things too much. People might *want* a form-feed in their
> messages, for all we know.

The patch was motivated by the integration of the wildmatch library, 
which exposes named character classes to users.  It replaces a call of 
fnmatch in match_pathname.  Users probably expect [:space:] to mean the 
same in git as in other programs.

I never saw a vertical tab and I can't imagine what it's used for.  I'd 
expect form-feeds to be matched as space, though.  Didn't see them very 
often, admittedly.

Nevertheless, it's unfortunate that we have an isspace() that *almost* 
does what the widely known thing of the same name does.  I'd shy away 
from changing git's version directly, because it's used more than a 
hundred times in the code, and estimating the impact of adding \v and \f 
to it.  Perhaps renaming it to isgitspace() is a good first step, 
followed by adding a "standard" version of isspace() for wildmatch?

René

^ permalink raw reply

* Re: [PATCHv3 3/4] git-status: show short sequencer state
From: Junio C Hamano @ 2012-11-14 19:35 UTC (permalink / raw)
  To: Phil Hord
  Cc: git, phil.hord, Jeff King, konglu, Matthieu Moy, Kong Lucien,
	Duperray Valentin, Jonas Franck, Nguy Thomas
In-Reply-To: <50A3ED8A.9080604@cisco.com>

Phil Hord <hordp@cisco.com> writes:

>   # tokens and short-status
>   $ git status --tree --short  
>   ## changed-files
>    M foo.txt

Hrm, how will the existing readers of the output avoid getting
confused by this overloading of "##", which has meant the current
branch information?

>   # tokens only?
>   $ git status --tree --porcelain
>   changed-files

It probably is simpler for parsers of the --porcelain format if you
(1) always used first two bytes as the record type, and (2) avoided
using "##" that is already used for the tree-wide status.

Just a thought.

^ permalink raw reply

* Re: [PATCH 4/6] var: provide explicit/implicit ident information
From: Jeff King @ 2012-11-14 19:53 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano
In-Reply-To: <20121114170657.GC6858@elie.Belkin>

On Wed, Nov 14, 2012 at 09:06:57AM -0800, Jonathan Nieder wrote:

> Jeff King wrote:
> 
> > Internally, we keep track of whether the author or committer
> > ident information was provided by the user, or whether it
> > was implicitly determined by the system. However, there is
> > currently no way for external programs or scripts to get
> > this information
> 
> What are the intended semantics?  If my machine has /etc/mailname
> filled out, is that an implicit identity?  How about if I set the
> EMAIL envvar but not GIT_COMMITTER_EMAIL?
> 
> If external scripts are going to start using this mechanism, they will
> need answers to these questions to support users that run into
> configuration problems.  A few words on this in the documentation
> could probably help.

The intent is to make git's internal rules (whatever they may be)
available to calling scripts. But I agree those rules should be spelled
out now that they are becoming a more visible interface. I'll update the
documentation.

> On most machines I have the EMAIL envvar set explicitly, but in the
> recent past I relied on /etc/mailname on some others, so I'm also
> genuinely curious about the use case here (and too lazy to dig it up).

Right now EMAIL is explicit, but `id`@`cat /etc/mailname` is not. I
think changing that would be a separate issue, though (and I'm not sure
whether it is a good idea; my /etc/mailname is not an email address I
would want to use).

-Peff

^ permalink raw reply

* Re: [PATCH 5/6] Git.pm: teach "ident" to query explicitness
From: Jeff King @ 2012-11-14 19:54 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano
In-Reply-To: <20121114171213.GD6858@elie.Belkin>

On Wed, Nov 14, 2012 at 09:12:13AM -0800, Jonathan Nieder wrote:

> > --- a/perl/Git.pm
> > +++ b/perl/Git.pm
> > @@ -737,7 +737,7 @@ sub remote_refs {
> >  }
> >  
> >  
> > -=item ident ( TYPE | IDENTSTR )
> > +=item ident ( TYPE | IDENTSTR [, options] )
> >  
> >  =item ident_person ( TYPE | IDENTSTR | IDENTARRAY )
> >  
> > @@ -750,6 +750,10 @@ and either returns it as a scalar string or as an array with the fields parsed.
> >  Alternatively, it can take a prepared ident string (e.g. from the commit
> >  object) and just parse it.
> >  
> > +If the C<explicit> option is set to 1, the returned array will contain an
> > +additional boolean specifying whether the ident was configure explicitly by the
> > +user.
> 
> s/configure/configured/
> 
> I'd suggest adding "See GIT_COMMITTER_EXPLICIT in git-var(1) for
> details" to make the semantics crystal clear.  What do you think?

Good suggestion. Thanks.

-Peff

^ permalink raw reply

* Re: [PATCHv3 3/4] git-status: show short sequencer state
From: Junio C Hamano @ 2012-11-14 19:57 UTC (permalink / raw)
  To: Phil Hord
  Cc: git, phil.hord, Jeff King, konglu, Matthieu Moy, Kong Lucien,
	Duperray Valentin, Jonas Franck, Nguy Thomas
In-Reply-To: <7v7gpoasf4.fsf@alter.siamese.dyndns.org>

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

> Phil Hord <hordp@cisco.com> writes:
>
>>   # tokens and short-status
>>   $ git status --tree --short  
>>   ## changed-files
>>    M foo.txt
>
> Hrm, how will the existing readers of the output avoid getting
> confused by this overloading of "##", which has meant the current
> branch information?

That was a stupid question.

Existing readers do not know to give --tree to the command, so they
are perfectly OK.

It is however not OK for new readers that wants to read these
tree-wide state and also branch information.

^ permalink raw reply

* Re: [PATCH 6/6] send-email: do not prompt for explicit repo ident
From: Jeff King @ 2012-11-14 20:05 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano
In-Reply-To: <20121114171827.GE6858@elie.Belkin>

On Wed, Nov 14, 2012 at 09:18:27AM -0800, Jonathan Nieder wrote:

> > The test scripts need to be adjusted to not expect a prompt
> > for the sender, since they always have the author explicitly
> > defined in the environment. Unfortunately, we cannot
> > reliably test that prompting still happens in the implicit
> > case, as send-email will produce inconsistent results
> > depending on the machine config (if we cannot find a FQDN,
> > "git var" will barf, causing us to exit early;
> 
> At first this sounded like a bug to me --- how could the user keep
> working without the sysadmin intervening?
> 
> But then I remembered that the user can set her name and email in
> .gitconfig and probably would want to in such a setup anyway.

Right. They would already have to to make commits, for example.

> When someone writes such a test, I think it could check that git
> either prompts or writes a message advising to configure the user
> email, no?  Waiting until later for that seems fine to me, though.

Yes. The problem is that the behavior and output are dependent on
factors outside the test suite, so we would have to check that one of
the possible expected outcomes happens. But I think there are really
only two such outcomes (neglecting that the ident itself can have
arbitrary content, but we do not have to check the actual content).

-Peff

^ permalink raw reply

* git init shared=group with a subdir
From: petesea @ 2012-11-14 20:00 UTC (permalink / raw)
  To: git

I believe there may be a bug when initializing a new repository relating 
to the directory permissions when the --shared=group option is used and 
the repository is within a sub-directory.

The following will create the "test.git" directory with 2775 permissions 
(which is as expected):

   $ ls -ld test.git
   ls: cannot access test.git: No such file or directory
   $ git init --bare --shared=group test.git
   Initialized empty shared Git repository in /tmp/test.git/
   $ ls -ld test.git
   drwxrwsr-x 7 pete users 4096 2012-11-14 11:15 test.git

This following will also create the "test.git" directory with 2775 
permissions, BUT the "subdir" directory ends up with 755 permissions:

   $ ls -ld subdir
   ls: cannot access subdir: No such file or directory
   $ git init --bare --shared=group subdir/test.git
   Initialized empty shared Git repository in /tmp/subdir/test.git/
   $ ls -ld subdir subdir/test.git
   drwxr-xr-x 3 pete users 4096 2012-11-14 11:16 subdir
   drwxrwsr-x 7 pete users 4096 2012-11-14 11:16 subdir/test.git

Assuming the "subdir" directory doesn't already exist and is created by 
the "git init" command AND the --shared=group option is used, then 
shouldn't the "subdir" directory also have 2775 permissions?

^ permalink raw reply

* Re: Bug? Subtree merge seems to choke on trailing slashes.
From: Junio C Hamano @ 2012-11-14 20:11 UTC (permalink / raw)
  To: Jack O'Connor; +Cc: git
In-Reply-To: <CA+6di1nsWOGXjchOc3SZ2FL79zGzgNqjOJyN0zf8Yb8GbXCpbA@mail.gmail.com>

"Jack O'Connor" <oconnor663@gmail.com> writes:

> Do I have the right list for bug reports? Apologies if not.

Yes.

> On Tue, Nov 6, 2012 at 5:58 PM, Jack O'Connor <oconnor663@gmail.com> wrote:
>>
>> 2) I try to merge more upstream changes on top of that with the
>> following command:
>> git merge --strategy-option=subtree='test/' $upstream_stuff
>> 3) Git fails with an obscure error:
>> fatal: entry  not found in tree daf4d0f0a20b8b6ec007be9fcafeac84a6eba4f0

I'd expect --strategy-option=subtree=./test, ././test (or ./test/
for that matter) would fail in a similar way, too.

What happens if you do not give an unnecessary '/' at the end?

If it works, I'd say that it falls into "dr. it hurts when I twist
my arm this way---don't do it then" category.

^ permalink raw reply

* Re: [PATCH 6/6] send-email: do not prompt for explicit repo ident
From: Jeff King @ 2012-11-14 20:26 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano
In-Reply-To: <20121114200505.GA13317@sigill.intra.peff.net>

On Wed, Nov 14, 2012 at 12:05:05PM -0800, Jeff King wrote:

> > When someone writes such a test, I think it could check that git
> > either prompts or writes a message advising to configure the user
> > email, no?  Waiting until later for that seems fine to me, though.
> 
> Yes. The problem is that the behavior and output are dependent on
> factors outside the test suite, so we would have to check that one of
> the possible expected outcomes happens. But I think there are really
> only two such outcomes (neglecting that the ident itself can have
> arbitrary content, but we do not have to check the actual content).

Actually, I think the simplest thing is to add a prerequisite, like:

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 489bc80..8d192ff 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -738,6 +738,14 @@ test_lazy_prereq UTF8_NFD_TO_NFC '
 	esac
 '
 
+test_lazy_prereq IMPLICIT_IDENT '
+	sane_unset GIT_AUTHOR_NAME &&
+	sane_unset GIT_AUTHOR_EMAIL &&
+	git var GIT_AUTHOR_IDENT &&
+	# double check that we were not polluted by config
+	test "$(git var GIT_AUTHOR_EXPLICIT)" = 0
+'
+
 # When the tests are run as root, permission tests will report that
 # things are writable when they shouldn't be.
 test -w / || test_set_prereq SANITY

We can't have one test machine that will cover all of the cases, but
given that the test suite is run by many people across many machines, we
will get coverage (and I know that some people have machines which would
not pass that prereq, because I got test failure reports during the last
ident refactoring).

I'll include something like that in my re-roll (and it should let us
test "git commit" more thoroughly, too).

-Peff

^ permalink raw reply related

* Re: git init shared=group with a subdir
From: Junio C Hamano @ 2012-11-14 20:29 UTC (permalink / raw)
  To: petesea; +Cc: git
In-Reply-To: <alpine.OSX.2.00.1211141107180.737@nikto-air>

petesea@bigfoot.com writes:

>   $ ls -ld subdir
>   ls: cannot access subdir: No such file or directory
>   $ git init --bare --shared=group subdir/test.git
>   Initialized empty shared Git repository in /tmp/subdir/test.git/
>   $ ls -ld subdir subdir/test.git
>   drwxr-xr-x 3 pete users 4096 2012-11-14 11:16 subdir
>   drwxrwsr-x 7 pete users 4096 2012-11-14 11:16 subdir/test.git
>
> Assuming the "subdir" directory doesn't already exist and is created
> by the "git init" command AND the --shared=group option is used, then
> shouldn't the "subdir" directory also have 2775 permissions?

People who access this "test.git" does not need to be able to write
into subdir/foobar, so the lack of w bit for the group is perfectly
fine, no?  Depending on to whom you would want to expose things
other than "test.git" in "subdir", the desirable set of the
permission bits on "subdir" itself would be different, but the "git
init" command line does not give us enough information to infer what
exact mode is needed.

At least we should give r-x to the parent directories for the
classes of users to whom we give rwx to the repository itself.  In
your example, since we give rwx to user=pete and group=users on
"test.git", "subdir" should have r-x (or better) for user=pete and
group=users, so that result happens to be correct.

But I do not think we did the right thing by design but it turned
out to be correct by accident.  If your umask were 077 or something
tight, I suspect that we end up creating "subdir" with "rwx------",
and group members wouldn't be able to access "test.git".  We may
want to loosen it down to "rwxr-x---" in such a case, but doing so
unconditionally risks exposing things inside "subdir" other than
"test.git" to your group members, and worse yet, we would be doing
so without telling the user.  If your umask is set to a tight value,
it is likely that you did so on purpose, and it is debatable if it
is a good thing to do.  It is safer to create these leading
directories like "subdir" honoring the umask, give the user a chance
to make sure that other things in "subdir" can be exposed to people
who have access to "test.git", and let the user handle the
permission (perhaps running "chmod g+rx subdir" as necessary).  And
that is the behaviour you are observing, I think.

^ permalink raw reply

* Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)
From: Torsten Bögershausen @ 2012-11-14 21:13 UTC (permalink / raw)
  To: Jeff King; +Cc: Mark Levedahl, Torsten Bögershausen, git
In-Reply-To: <20121114190228.GA3860@sigill.intra.peff.net>

On 14.11.12 20:02, Jeff King wrote:
> On Tue, Nov 13, 2012 at 08:18:53PM -0500, Mark Levedahl wrote:
> 
>> On 11/13/2012 03:45 PM, Torsten Bögershausen wrote:
>>>> * ml/cygwin-mingw-headers (2012-11-12) 1 commit
>>>>  - Update cygwin.c for new mingw-64 win32 api headers
>>>>
>>>>  Make git work on newer cygwin.
>>>>
>>>>  Will merge to 'next'.
>>> (Sorry for late answer, I managed to test the original patch minutes before Peff merged it to pu)
>>> (And thanks for maintaining git)
>>>
>>> Is everybody using cygwin happy with this?
>>>
>>> I managed to compile on a fresh installed cygwin,
>>> but failed to compile under 1.7.7, see below.
>>> Is there a way we can achieve to compile git both under "old" and "new" cygwin 1.7 ?
>>> Or is this not worth the effort?
>>>
>> I found no version info defined that could be used to automatically
>> switch between the old and current headers. You can always
>>
>>     make V15_MINGW_HEADERS=1 ...
>>
>> to force using the old set if you do not wish to update your installation.
> 
> Should we keep the code change, then, but not flip the default (i.e.,
> make people on the newer version opt into it)? I am not clear on how
> common the newer include system is. Of course, auto-detecting would be
> the ideal.
> 
> -Peff
There are a couple of things which we may want consider:
a) the name V15_MINGW_HEADERS:
  It indicates that this is true for Version 1.5 (of what?)
  If I assume Cygwin version 1.5 , then this name is confusing.
  Even cygwin versions like 1.7.7 use the same (or similar) include files as 1.5
  A better name could be CYGWIN_USE_MINGW_HEADERS (or the like) and to revert the logic.

b) Autodetection:
  (Just loud thinking), running 
$grep mingw /usr/include/w32api/winsock2.h
 * This file is part of the mingw-w64 runtime package.
#include <_mingw_unicode.h>

on cygwin 1.7.17 indicates that we can use grep in the Makefile to autodetect the "mingw headers"

Something like this in Makefile:
+ifeq ($(shell grep mingw /usr/include/w32api/winsock2.h />/dev/null 2>/dev/null && echo y),y)
+	CYGWIN_USE_MINGW_HEADERS=YesPlease
+endif

c) I'm not sure if we want to change cygwin.c or git-compat-util.h for this.

I can prepare a proper patch within the next couple of days

/Torsten







 

^ permalink raw reply

* Re: creation of empty branches
From: Junio C Hamano @ 2012-11-14 21:27 UTC (permalink / raw)
  To: Andrew Ardill; +Cc: Angelo Borsotti, git
In-Reply-To: <CAH5451mkcszgJxziKn3q3OwSDM-qQ71PtT5+UWb=PG7VYAcFyQ@mail.gmail.com>

Andrew Ardill <andrew.ardill@gmail.com> writes:

> Since git branch has the default behaviour to create a branch 'in the
> background' it makes sense to fail when trying to create a new branch
> this way from an empty branch. The error message should be improved to
> handle this edge case in a nicer way. If we allow for renaming empty
> branches (described below) then the message can be even more helpful.
> Instead of
>     fatal: Not a valid object name: 'master'.
> perhaps
>     fatal: Cannot create branch 'foo' from empty branch 'master'. To
> rename 'master' use 'git branch -m master foo'.

The first new sentence is a definite improvement, but I do not think
the advice in the second sentence is necessarily a good idea,
because it is dubious that the user is likely to have wanted to
rename 'master' to something else.  "git branch foo master" (or its
moral equivalent "git checkout -b foo" while on master) is a wish to
have a history that ends in 'foo' *forked* from history of 'master',
but because you do not even have anything on 'master' yet, you
cannot fork the history, as you explained earlier (snipped).  In
that sense, 'empty branch' is a slight misnomer---as far as "git
branch foo master" is concerned, the 'master' branch does not yet
exist (and that is why we often call it an "unborn branch", not
"empty").

    fatal: cannot fork master's history that does not exist yet.

would be more accurate description of the situation.

> So explicitly, I am proposing the following behaviour changes:
>
> When trying to create a new branch without specifying a start point,
> if HEAD points to an empty branch, error with a more useful message
> that assumes the user might want to rename the empty branch.

I do not think that is the right assumption in the first place.  It
is very likely that the user does not yet know how Git works when
she attempts to fork from a history that does not exist.  It is also
very likely that she is expecting, after "git branch foo master"
succeeds when 'master' is yet to be born, have two branches 'foo'
and 'master', so that "git checkout foo" and "git checkout master"
can be done subsequently.

But that expectation is wrong, and it would help the user in the
longer run to correct that expectation.  "We assume you wanted to
rename 'master' to 'foo'" is a logical consequence that changing
HEAD that points at an unborn 'master' to point at an unborn 'foo'
is the best (or closest) thing the user can do, *if* the user
understands that the current branch being unborn is a special state
and there can only be one such unborn branch (that is, the current
one).  The user who gets this error message, however, clearly does
not understand that, so it is not a logical consequence to her at
all.  The advice does not help her, but instead invites "No, I did
not want to rename it, I wanted to have 'foo' without losing
'master'", leading to even more confusion.

> When trying to create a new branch whilst specifying an empty branch
> as the start point,
>   if HEAD points to the same empty branch that is listed as the start
> point, error with a more useful message that assumes the user might
> want to rename the empty branch.
>   otherwise error due to invalid ref

See above (for all the other cases, too).

^ 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