Git development
 help / color / mirror / Atom feed
* Re: [PATCH v1] travis-ci: fix Perforce install on macOS
From: Junio C Hamano @ 2017-01-23 18:22 UTC (permalink / raw)
  To: larsxschneider; +Cc: git
In-Reply-To: <20170121144245.31702-1-larsxschneider@gmail.com>

larsxschneider@gmail.com writes:

> Could you fast track the patch to `maint` if it works without trouble on
> `next` (as it should!)?
>
> Notes:
>     Base Commit: 787f75f056 (master)

I do not think there is any difference between 'maint' and 'master'
for this file right now, but I still would have appreciated if this
line also said 'maint', not 'master', if you want it to go to
'maint' eventually ;-) As https://travis-ci.org/git/git/builds seems
to be doing 'pu', too, hopefully we'll catch any issues there first.

Thanks.

>     Diff on Web: https://github.com/larsxschneider/git/commit/ec7106339d
>     Checkout:    git fetch https://github.com/larsxschneider/git travisci/brew-perforce-fix-v1 && git checkout ec7106339d
>
>  .travis.yml | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 3843967a69..c6ba8c8ec5 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -75,20 +75,13 @@ before_install:
>        popd
>        ;;
>      osx)
> -      brew_force_set_latest_binary_hash () {
> -        FORMULA=$1
> -        SHA=$(brew fetch --force $FORMULA 2>&1 | grep ^SHA256: | cut -d ' ' -f 2)
> -        sed -E -i.bak "s/sha256 \"[0-9a-f]{64}\"/sha256 \"$SHA\"/g" \
> -          "$(brew --repository homebrew/homebrew-binary)/$FORMULA.rb"
> -      }
>        brew update --quiet
>        brew tap homebrew/binary --quiet
> -      brew_force_set_latest_binary_hash perforce
> -      brew_force_set_latest_binary_hash perforce-server
>        # Uncomment this if you want to run perf tests:
>        # brew install gnu-time
> -      brew install git-lfs perforce-server perforce gettext
> +      brew install git-lfs gettext
>        brew link --force gettext
> +      brew install Caskroom/cask/perforce
>        ;;
>      esac;
>      echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)";
> --
> 2.11.0

^ permalink raw reply

* Re: [PATCH 2/3] stash: introduce push verb
From: Junio C Hamano @ 2017-01-23 18:27 UTC (permalink / raw)
  To: Thomas Gummerer
  Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin
In-Reply-To: <20170121200804.19009-3-t.gummerer@gmail.com>

Thomas Gummerer <t.gummerer@gmail.com> writes:

> +	stash_msg="$*"
> +
> +	if test -z stash_msg

A dollar-sign is missing here, I think.

> +	then
> +		push_stash $push_options
> +	else
> +		push_stash $push_options -m "$stash_msg"
> +	fi
> +}

^ permalink raw reply

* Re: [PATCH v1] travis-ci: fix Perforce install on macOS
From: Lars Schneider @ 2017-01-23 18:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq8tq1tz49.fsf@gitster.mtv.corp.google.com>


> On 23 Jan 2017, at 19:22, Junio C Hamano <gitster@pobox.com> wrote:
> 
> larsxschneider@gmail.com writes:
> 
>> Could you fast track the patch to `maint` if it works without trouble on
>> `next` (as it should!)?
>> 
>> Notes:
>>    Base Commit: 787f75f056 (master)
> 
> I do not think there is any difference between 'maint' and 'master'
> for this file right now, but I still would have appreciated if this
> line also said 'maint', not 'master', if you want it to go to
> 'maint' eventually ;-) As https://travis-ci.org/git/git/builds seems
> to be doing 'pu', too, hopefully we'll catch any issues there first.

You're right! My own automation betrayed me :-)

I made a tiny change, though. Can you queue v2?
http://public-inbox.org/git/20170122225550.28422-1-larsxschneider@gmail.com/

Thanks,
Lars


^ permalink raw reply

* Re: [RFC] Case insensitive Git attributes
From: Junio C Hamano @ 2017-01-23 18:35 UTC (permalink / raw)
  To: Dakota Hawkins
  Cc: Duy Nguyen, Johannes Schindelin, Stefan Beller, Lars Schneider,
	git
In-Reply-To: <CAHnyXxRx_iagZhki1rmVEpw+GZPWBRx7mNmahs3pruy57L3h-Q@mail.gmail.com>

Dakota Hawkins <dakota@dakotahawkins.com> writes:

> Apologies for the delayed bump. I think because we're talking about
> affecting the behavior of .gitattributes that it would be better to
> have a distinct .gitattributes option, whether or not you also have a
> similar config option.

As I know I am on the To: line of the message I am responding to,
let me quicly let you know that I won't be responding to this thread
for a while as I don't recall what the discussion was about.  I will
after I'll dig and find out what the thread was about but it won't
happen immediately.  Sorry about that.

^ permalink raw reply

* Re: [PATCH] blame: add option to print tips (--tips)
From: Junio C Hamano @ 2017-01-23 18:36 UTC (permalink / raw)
  To: Pranit Bauva; +Cc: Edmundo Carmona Antoranz, Git List
In-Reply-To: <CAFZEwPPx2vDJVf=uk0iUJ2sh9DxWwp2Lp1k-APz9n=7NYMN5uQ@mail.gmail.com>

Pranit Bauva <pranit.bauva@gmail.com> writes:

> We can probably make it useful with some extended efforts. I use
> git-blame and I sometimes find that I don't need things like the name
> of the author, time, timezone and not even the file name and I have to
> use a bigger terminal. If we could somehow remove those fields then
> maybe this would be a useful feature.

I admit that I didn't recall the option until somebody else told me,
but I think "blame -s" or something like that for that purpose ;-)


^ permalink raw reply

* Re: [PATCH v2] travis-ci: fix Perforce install on macOS
From: Junio C Hamano @ 2017-01-23 18:39 UTC (permalink / raw)
  To: Lars Schneider; +Cc: git
In-Reply-To: <20170122225550.28422-1-larsxschneider@gmail.com>

Lars Schneider <larsxschneider@gmail.com> writes:

> The `perforce` and `perforce-server` package were moved from brew [1][2]
> to cask [3]. Teach TravisCI the new location.
>
> Perforce updates their binaries without version bumps. That made the
> brew install (legitimately!) fail due to checksum mismatches. The
> workaround is not necessary anymore as Cask [4] allows to disable the
> checksum test for individual formulas.
>
> [1] https://github.com/Homebrew/homebrew-binary/commit/1394e42de04d07445f82f9512627e864ff4ca4c6
> [2] https://github.com/Homebrew/homebrew-binary/commit/f8da22d6b8dbcfcfdb2dfa9ac1a5e5d8e05aac2b
> [3] https://github.com/caskroom/homebrew-cask/pull/29180
> [4] https://caskroom.github.io/
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
>
> Hi,
>
> this small update removes one more unnecessary line and makes the
> formula name lower case (no functional reason - just looks better ;-).
>
> @Junio: Do you prefer such a v2 as "--in-reply-to" to v1 or as separate
>         thread? What eases your workflow?

It does not make that much of a difference if the second one comes
within a few days since the first one but in general it probably
would help if the follow-up is threaded _and_ made it clear upfront
that if somebody is reading v2 then v1 can be ignored ;-).

Thanks.

^ permalink raw reply

* Re: [RFC] Case insensitive Git attributes
From: Lars Schneider @ 2017-01-23 18:42 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Dakota Hawkins, Duy Nguyen, Johannes Schindelin, Stefan Beller,
	git
In-Reply-To: <xmqqvat5sjym.fsf@gitster.mtv.corp.google.com>


> On 23 Jan 2017, at 19:35, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Dakota Hawkins <dakota@dakotahawkins.com> writes:
> 
>> Apologies for the delayed bump. I think because we're talking about
>> affecting the behavior of .gitattributes that it would be better to
>> have a distinct .gitattributes option, whether or not you also have a
>> similar config option.
> 
> As I know I am on the To: line of the message I am responding to,
> let me quicly let you know that I won't be responding to this thread
> for a while as I don't recall what the discussion was about.  I will
> after I'll dig and find out what the thread was about but it won't
> happen immediately.  Sorry about that.


Problem:
Git attributes for path names are generally case sensitive. However, on 
a case insensitive file system (e.g. macOS/Windows) they appear to be
case insensitive (`*.bar` would match `foo.bar` and `foo.BAR`). That 
works great until a Git users joins the party with a case sensitive file 
system. For this Git user the attributes pattern only matches files with
the exact case (*.bar` would match only `foo.bar`).

Question/Proposal:
Could we introduce some flag to signal that certain attribute patterns
are always case insensitive? 

Thread:
http://public-inbox.org/git/C83BE22D-EAC8-49E2-AEE3-22D4A99AE205@gmail.com/#t

Cheers,
Lars

^ permalink raw reply

* Re: [PATCH 3/3] stash: support filename argument
From: Junio C Hamano @ 2017-01-23 18:50 UTC (permalink / raw)
  To: Thomas Gummerer
  Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin
In-Reply-To: <20170121200804.19009-4-t.gummerer@gmail.com>

Thomas Gummerer <t.gummerer@gmail.com> writes:

> diff --git a/git-stash.sh b/git-stash.sh
> index d6b4ae3290..7dcce629bd 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -41,7 +41,7 @@ no_changes () {
>  untracked_files () {
>  	excl_opt=--exclude-standard
>  	test "$untracked" = "all" && excl_opt=
> -	git ls-files -o -z $excl_opt
> +	git ls-files -o -z $excl_opt -- $1

Does $1 need to be quoted to prevent it from split at $IFS?

> @@ -56,6 +56,23 @@ clear_stash () {
>  }
>  
>  create_stash () {
> +	files=
> +	while test $# != 0
> +	do
> +		case "$1" in
> +		--)
> +			shift
> +			break
> +			;;
> +		--files)
> +			;;
> +		*)
> +			files="$1 $files"
> +			;;

Hmph.  What is this "no-op" option about?  Did you mean to say
something like this instead?

	case "$1" in
	...
	--file)
		case $# in
		1)
			die "--file needs a pathspec" ;;
		*)
			shift
			files="$files$1 " ;;
		esac
		;;


Another thing I noticed.  We won't support filenames with embedded
$IFS characters at all?

I somehow had an impression that the script was carefully done
(e.g. by using -z option where appropriate) to add such a
limitation.

Perhaps we have broken it over time and it no longer matters
(i.e. there already may be existing breakages), but this troubles
me somehow.

By the way, in addition to "push" thing that corrects the argument
convention by requiring "-m" before the message, we need to correct
create_stash that is used internally from "stash push" somehow?

^ permalink raw reply

* Re: [PATCH v3 14/21] read-cache: touch shared index files when used
From: Junio C Hamano @ 2017-01-23 18:53 UTC (permalink / raw)
  To: Christian Couder
  Cc: Duy Nguyen, git, Ævar Arnfjörð Bjarmason,
	Christian Couder
In-Reply-To: <CAP8UFD2LWGtNtdhtQTZXqsACBvK=jD25vt8M4HzBRBVS1sJ+=Q@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> writes:

> Also in general the split-index mode is useful when you often write
> new indexes, and in this case shared index files that are used will
> often be freshened, so the risk of deleting interesting shared index
> files should be low.
> ...
>> Not that I think freshening would actually fail in a repository
>> where you can actually write into to update the index or its refs to
>> make a difference (iow, even if we make it die() loudly when shared
>> index cannot be "touched" because we are paranoid, no real life
>> usage will trigger that die(), and if a repository does trigger the
>> die(), I think you would really want to know about it).
>
> As I wrote above, I think if we can actually write the shared index
> file but its freshening fails, it probably means that the shared index
> file has been removed behind us, and this case is equivalent as when
> loose files have been removed behind us.

OK, so it is unlikely to happen, and when it happens it leads to a
catastrophic failure---do we just ignore or do we report an error?

^ permalink raw reply

* Re: [PATCH v2 0/7] Macros for Asciidoctor support
From: Junio C Hamano @ 2017-01-23 18:59 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Johannes Schindelin, Jeff King
In-Reply-To: <20170122024156.284180-1-sandals@crustytoothpaste.net>

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> There are two major processors of AsciiDoc: AsciiDoc itself, and Asciidoctor.
> Both have advantages and disadvantages, but traditionally the documentation has
> been built with AsciiDoc, leading to some surprising breakage when building with
> Asciidoctor.  Partially, this is due to the need to specify a significant number
> of macros on the command line when building with Asciidoctor.
>
> This series cleans up some issues building the documentation with Asciidoctor
> and provides two knobs, USE_ASCIIDOCTOR, which controls building with
> Asciidoctor, and ASCIIDOCTOR_EXTENSIONS_LAB, which controls the location of the
> Asciidoctor Extensions Lab, which is necessary to expand the linkgit macro.
>
> The need for the extensions could be replaced with a small amount of Ruby code,
> if that's considered desirable.  Previous opinions on doing so were negative,
> however.
>
> In the process, I found several issues with cat-texi.perl, which have been
> fixed.  It has also been modernized to use strict, warnings, and lexical file
> handles.  I also made an attempt to produce more diffable texi files; I may
> follow up with additional series along this line to make the documentation build
> reproducibly.

Thanks.  We'd probably want INSTALL to talk about Asciidoctor once
this matures, as it is very simple requirement for the builder to
have to just set USE_ASCIIDOCTOR, but the version requirement and
stuff might be still confusing.

^ permalink raw reply

* Re: [DEMO][PATCH v2 6/5] compat: add a qsort_s() implementation based on GNU's qsort_r(1)
From: Junio C Hamano @ 2017-01-23 19:07 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git List, Jeff King, Johannes Schindelin
In-Reply-To: <9f8b564d-ec9f-abc9-77f6-aa84c6e78b7a@web.de>

René Scharfe <l.s.r@web.de> writes:

> Implement qsort_s() as a wrapper to the GNU version of qsort_r(1) and
> use it on Linux.  Performance increases slightly:
>
> Test                         HEAD^             HEAD
> --------------------------------------------------------------------
> 0071.2: sort(1)              0.10(0.20+0.02)   0.10(0.21+0.01) +0.0%
> 0071.3: string_list_sort()   0.17(0.15+0.01)   0.16(0.15+0.01) -5.9%
>
> Additionally the unstripped size of compat/qsort_s.o falls from 24576
> to 16544 bytes in my build.
>
> IMHO these savings aren't worth the increased complexity of having to
> support two implementations.

I do worry about having to support more implementations in the
future that have different function signature for the comparison
callbacks, which will make things ugly, but this addition alone
doesn't look too bad to me.

Thanks.

> diff --git a/compat/qsort_s.c b/compat/qsort_s.c
> index 52d1f0a73d..763ee1faae 100644
> --- a/compat/qsort_s.c
> +++ b/compat/qsort_s.c
> @@ -1,5 +1,21 @@
>  #include "../git-compat-util.h"
>  
> +#if defined HAVE_GNU_QSORT_R
> +
> +int git_qsort_s(void *b, size_t n, size_t s,
> +		int (*cmp)(const void *, const void *, void *), void *ctx)
> +{
> +	if (!n)
> +		return 0;
> +	if (!b || !cmp)
> +		return -1;
> +
> +	qsort_r(b, n, s, cmp, ctx);
> +	return 0;
> +}
> +
> +#else
> +
>  /*
>   * A merge sort implementation, simplified from the qsort implementation
>   * by Mike Haertel, which is a part of the GNU C Library.
> @@ -67,3 +83,5 @@ int git_qsort_s(void *b, size_t n, size_t s,
>  	}
>  	return 0;
>  }
> +
> +#endif
> diff --git a/config.mak.uname b/config.mak.uname
> index 447f36ac2e..a1858f54ff 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -37,6 +37,7 @@ ifeq ($(uname_S),Linux)
>  	NEEDS_LIBRT = YesPlease
>  	HAVE_GETDELIM = YesPlease
>  	SANE_TEXT_GREP=-a
> +	HAVE_GNU_QSORT_R = YesPlease
>  endif
>  ifeq ($(uname_S),GNU/kFreeBSD)
>  	HAVE_ALLOCA_H = YesPlease

^ permalink raw reply

* Re: [RFC] Case insensitive Git attributes
From: Junio C Hamano @ 2017-01-23 19:25 UTC (permalink / raw)
  To: Lars Schneider
  Cc: Dakota Hawkins, Duy Nguyen, Johannes Schindelin, Stefan Beller,
	git
In-Reply-To: <C265204B-BCCF-4085-9933-F28EB459CFB9@gmail.com>

Lars Schneider <larsxschneider@gmail.com> writes:

> Problem:
> Git attributes for path names are generally case sensitive. However, on 
> a case insensitive file system (e.g. macOS/Windows) they appear to be
> case insensitive (`*.bar` would match `foo.bar` and `foo.BAR`). That 
> works great until a Git users joins the party with a case sensitive file 
> system. For this Git user the attributes pattern only matches files with
> the exact case (*.bar` would match only `foo.bar`).
>
> Question/Proposal:
> Could we introduce some flag to signal that certain attribute patterns
> are always case insensitive? 
>
> Thread:
> http://public-inbox.org/git/C83BE22D-EAC8-49E2-AEE3-22D4A99AE205@gmail.com/#t

Thanks for a pointer.

So you are worried about the case where somebody on a case
insensitive but case preserving system would do

    $ edit file.txt
    $ edit .gitattributes
    $ git add file.txt .gitattributes

and adds "*.TXT	someattr=true" to the attributes file, which
would set someattr to true on his system for file.txt, but when the
result is checked out on a case sensitive system, it would behave
differently because "*.TXT" does not match "file.txt"?

How do other systems address it?  Your Java, Ruby, etc. sources may
refer to another file with "import" and the derivation of the file
names from class names or package names would have the same issue,
isn't it?  Do they have an option that lets you say

    Even though the import statements may say "import a.b.C", we
    know that the source tarball was prepared on a case insensitive
    system, and I want you to look for a/b/C.java and a/b/c.java and
    use what was found.

or something like that?  Same for anything that records other
filenames in the content to refer to them, like "Makefile".

My knee jerk reaction to that is that .gitattributes and .gitignore
should not be instructed to go case insensitive on case sensitive
systems.  If the system is case insensitive but case preserving,
it probably would make sense not to do case insensitive matching,
which would prevent the issue from happening in the first place.



^ permalink raw reply

* Re: [RFC] Case insensitive Git attributes
From: Junio C Hamano @ 2017-01-23 19:38 UTC (permalink / raw)
  To: Lars Schneider
  Cc: Dakota Hawkins, Duy Nguyen, Johannes Schindelin, Stefan Beller,
	git
In-Reply-To: <xmqq1svtshnr.fsf@gitster.mtv.corp.google.com>

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

> So you are worried about the case where somebody on a case
> insensitive but case preserving system would do
>
>     $ edit file.txt
>     $ edit .gitattributes
>     $ git add file.txt .gitattributes
>
> and adds "*.TXT	someattr=true" to the attributes file, which
> would set someattr to true on his system for file.txt, but when the
> result is checked out on a case sensitive system, it would behave
> differently because "*.TXT" does not match "file.txt"?
>
> How do other systems address it?  Your Java, Ruby, etc. sources may
> refer to another file with "import" and the derivation of the file
> names from class names or package names would have the same issue,
> isn't it?  Do they have an option that lets you say
>
>     Even though the import statements may say "import a.b.C", we
>     know that the source tarball was prepared on a case insensitive
>     system, and I want you to look for a/b/C.java and a/b/c.java and
>     use what was found.
>
> or something like that?  Same for anything that records other
> filenames in the content to refer to them, like "Makefile".
>
> My knee jerk reaction to that is that .gitattributes and .gitignore
> should not be instructed to go case insensitive on case sensitive
> systems.  If the system is case insensitive but case preserving,
> it probably would make sense not to do case insensitive matching,
> which would prevent the issue from happening in the first place.

Sorry, but there is a slight leap in the above that makes it hard to
track my thought, so let me clarify a bit.  

In the above, I am guessing the answer to the "How do other systems
address it?" question to be "nothing".  And that leads to the
conclusion that it is better to do "nothing on case sensitive
systems, and probably become evem more strict on case insensitive
but case preserving systems", because that will give us a chance to
expose the problem earlier, hopefully even on the originating
system.

^ permalink raw reply

* Re: [PATCH v1 0/2] urlmatch: allow regexp-based matches
From: Junio C Hamano @ 2017-01-23 19:53 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Patrick Steinhardt
In-Reply-To: <20170123130635.29577-1-patrick.steinhardt@elego.de>

Patrick Steinhardt <patrick.steinhardt@elego.de> writes:

> This patch is mostly a request for comments. The use case is to
> be able to configure an HTTP proxy for all subdomains of a
> certain domain where there are hundreds of subdomains. The most
> flexible way I could imagine was by using regular expressions for
> the matching, which is how I implemented it for now. So users can
> now create a configuration key like
> `http.?http://.*\\.example\\.com.*` to apply settings for all
> subdomains of `example.com`.

While reading 2/2, I got an impression that this is "too" flexible
and possibly operates at a wrong level.  I would have expected that
the wildcarding to be limited to the host part only and hook into
match_urls(), allowing the users of the new feature to still take
advantage of the existing support of "http://me@example.com" that
limits the match to the case that the connection is authenticated
for a user, for example, by newly allowing "http://me@*.example.com"
or something like that.

Because you cannot have a literal '*' in your hostname, I would
imagine that supporting a match pattern "http://me@*.example.com"
would be already backward compatible without requiring a leading
question-mark.

I also personally would prefer these textual matching to be done
with glob not with regexp, by the way, as the above description of
mine shows.

Thanks.



^ permalink raw reply

* Re: [PATCH v3 08/21] Documentation/git-update-index: talk about core.splitIndex config var
From: Junio C Hamano @ 2017-01-23 19:59 UTC (permalink / raw)
  To: Christian Couder
  Cc: Duy Nguyen, git, Ævar Arnfjörð Bjarmason,
	Christian Couder
In-Reply-To: <CAP8UFD2aQJ92KjzTQTGLyYeEuVk9TK51mn05OSWCZu5c4c6WuQ@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> writes:

>>> Perhaps we should declare that config will be the one and only way
>>> in the future and start deprecating the command line option way.
>>> That will remove the need for two to interact with each other.
>
> That would be my preferred solution as I think it is the simplest in
> the end for users.
> Also, as Duy wrote above, one can always use something like "git -c
> core.splitIndex= ...", which by the way can work for any command, not
> just "update-index".
>
> Anyway we would have to introduce core.splitIndex first, and it
> wouldn't make sense to have a different behavior between
> --[no-]split-index and --[no-]untracked-cache in the meantime before
> they are deprecated and eventually removed.
>
> So let's just go with the implementation in this series, which is
> similar to --[no-]untracked-cache, and let's plan to deprecate
> --[no-]split-index and --[no-]untracked-cache in a later patch series.

Sounds like we have a plan.  Thanks.

^ permalink raw reply

* Re: [PATCH] rebase: pass --signoff option to git am
From: Giuseppe Bilotta @ 2017-01-23 20:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List
In-Reply-To: <xmqqh94ptzke.fsf@gitster.mtv.corp.google.com>

On Mon, Jan 23, 2017 at 7:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Should we plan to extend this to the interactive backend that is
> shared between rebase -i and rebase -m, too?  Or is this patch
> already sufficient to cover them?

AFAIK this is sufficient for both, in the sense that I've used it with
git rebase -i and it works.

^ permalink raw reply

* Re: [PATCH v3 0/5] show-ref: Allow -d, --head to work with --verify
From: Junio C Hamano @ 2017-01-23 20:03 UTC (permalink / raw)
  To: Vladimir Panteleev; +Cc: git
In-Reply-To: <20170123180059.4288-1-git@thecybershadow.net>

Vladimir Panteleev <git@thecybershadow.net> writes:

> Third iteration, according to Junio's comments. This time we keep
> show_ref and show_one separate, accept HEAD with --verify even without
> --head, and add tests for dangling ref validation with --verify.

I am no longer a neutral judge but to me the resulting code looks a
lot more naturally structured than the original.

Thanks.  Will queue.

^ permalink raw reply

* Re: [RFC PATCH] Option to allow cherry-pick to skip empty commits
From: Giuseppe Bilotta @ 2017-01-23 20:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List
In-Reply-To: <xmqqd1fdtzgv.fsf@gitster.mtv.corp.google.com>

On Mon, Jan 23, 2017 at 7:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
>
>> By the way, I noticed going over the code that the -allow options are
>> not stored, so that in case of interruption they will be reset, is
>> this intentional or a bug?
>
> I do not know offhand, but given the history of the two commands, I
> would guess it was a bug simply overlooked when people bolted "a
> series of commits" mode onto these commands that originally worked
> only on a single commit.

It seems like a bug to me. I'll prepare a new series that also fixes
this. I still don't see how to force a complete reread of the index
after running a git reset (which I need for the --skip command), but
maybe I'll add a WIP of what I have for --skip so far and people can
comment on that.


-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply

* Re: [RFC PATCH] Option to allow cherry-pick to skip empty commits
From: Junio C Hamano @ 2017-01-23 20:10 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: Git List
In-Reply-To: <CAOxFTcxRjWS-=wyGBVOtg-tfCHrqqAr9rVOcvkyxXwJHonN_Tg@mail.gmail.com>

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:

> ... I still don't see how to force a complete reread of the index
> after running a git reset (which I need for the --skip command), ...

Do you mean discard_index() or discard_cache() followed by
read_index() or read_cache(), or do you mean something more
elaborate?

^ permalink raw reply

* Re: [PATCH] rebase: pass --signoff option to git am
From: Junio C Hamano @ 2017-01-23 20:16 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: Git List
In-Reply-To: <CAOxFTcyuLkvgPOxQuzaDUVuDRu_KJg=JrYtU84pQyjLstChbLg@mail.gmail.com>

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:

> On Mon, Jan 23, 2017 at 7:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Should we plan to extend this to the interactive backend that is
>> shared between rebase -i and rebase -m, too?  Or is this patch
>> already sufficient to cover them?
>
> AFAIK this is sufficient for both, in the sense that I've used it with
> git rebase -i and it works.

That is a good news and at the same time a bit awkard one ;-)  

The mention of "passed to 'git am'" twice in the documentation and
help text would lead people to think "rebase -i" would not be
affected and (1) would need more work to do so, or (2) the user does
not want "rebase -i" to be unaffected for whatever reason, and gets
surprised to see that it actually does get affected.

In any case, will queue as-is so that we won't lose the patch while
waiting for people to raise their opinions.

Thanks.

^ permalink raw reply

* [PATCH v2 00/27] Revamp the attribute system; another round
From: Brandon Williams @ 2017-01-23 20:34 UTC (permalink / raw)
  To: git; +Cc: Brandon Williams, sbeller, gitster, pclouds
In-Reply-To: <20170112235354.153403-1-bmwill@google.com>

Changes in v2:
* surround the mutex initializer calls by #ifdef
* mark file-local symbol static
* handling of attribute stacks.  Instead of storing each stack frame in a
  hashmap, there is a stack per attr_check instance.  This will allow for
  easier optimizing of the stack in future patches as well as eliminates the
  potential for memory to grow unbounded.  This is also more inline with the
  original vision of the attribute system refactor.

Brandon Williams (8):
  attr: pass struct attr_check to collect_some_attrs
  attr: use hashmap for attribute dictionary
  attr: eliminate global check_all_attr array
  attr: remove maybe-real, maybe-macro from git_attr
  attr: tighten const correctness with git_attr and match_attr
  attr: store attribute stack in attr_check structure
  attr: push the bare repo check into read_attr()
  attr: reformat git_attr_set_direction() function

Junio C Hamano (17):
  commit.c: use strchrnul() to scan for one line
  attr.c: use strchrnul() to scan for one line
  attr.c: update a stale comment on "struct match_attr"
  attr.c: explain the lack of attr-name syntax check in parse_attr()
  attr.c: complete a sentence in a comment
  attr.c: mark where #if DEBUG ends more clearly
  attr.c: simplify macroexpand_one()
  attr.c: tighten constness around "git_attr" structure
  attr.c: plug small leak in parse_attr_line()
  attr.c: add push_stack() helper
  attr.c: outline the future plans by heavily commenting
  attr: rename function and struct related to checking attributes
  attr: (re)introduce git_check_attr() and struct attr_check
  attr: convert git_all_attrs() to use "struct attr_check"
  attr: convert git_check_attrs() callers to use the new API
  attr: retire git_check_attrs() API
  attr: change validity check for attribute names to use positive logic

Nguyễn Thái Ngọc Duy (1):
  attr: support quoting pathname patterns in C style

Stefan Beller (1):
  Documentation: fix a typo

 Documentation/gitattributes.txt               |  10 +-
 Documentation/technical/api-gitattributes.txt |  86 ++-
 archive.c                                     |  24 +-
 attr.c                                        | 854 ++++++++++++++++++--------
 attr.h                                        |  53 +-
 builtin/check-attr.c                          |  66 +-
 builtin/pack-objects.c                        |  19 +-
 commit.c                                      |   3 +-
 common-main.c                                 |   3 +
 convert.c                                     |  25 +-
 ll-merge.c                                    |  33 +-
 t/t0003-attributes.sh                         |  26 +
 userdiff.c                                    |  19 +-
 ws.c                                          |  19 +-
 14 files changed, 800 insertions(+), 440 deletions(-)

-- 
2.11.0.483.g087da7b7c-goog


^ permalink raw reply

* [PATCH v2 01/27] commit.c: use strchrnul() to scan for one line
From: Brandon Williams @ 2017-01-23 20:34 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, sbeller, pclouds, Brandon Williams
In-Reply-To: <20170123203525.185058-1-bmwill@google.com>

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

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
 commit.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/commit.c b/commit.c
index 2cf85158b..0c4ee3de4 100644
--- a/commit.c
+++ b/commit.c
@@ -415,8 +415,7 @@ int find_commit_subject(const char *commit_buffer, const char **subject)
 		p++;
 	if (*p) {
 		p = skip_blank_lines(p + 2);
-		for (eol = p; *eol && *eol != '\n'; eol++)
-			; /* do nothing */
+		eol = strchrnul(p, '\n');
 	} else
 		eol = p;
 
-- 
2.11.0.483.g087da7b7c-goog


^ permalink raw reply related

* [PATCH v2 02/27] attr.c: use strchrnul() to scan for one line
From: Brandon Williams @ 2017-01-23 20:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, sbeller, pclouds, Brandon Williams
In-Reply-To: <20170123203525.185058-1-bmwill@google.com>

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

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
 attr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/attr.c b/attr.c
index 1fcf042b8..04d24334e 100644
--- a/attr.c
+++ b/attr.c
@@ -402,8 +402,8 @@ static struct attr_stack *read_attr_from_index(const char *path, int macro_ok)
 	for (sp = buf; *sp; ) {
 		char *ep;
 		int more;
-		for (ep = sp; *ep && *ep != '\n'; ep++)
-			;
+
+		ep = strchrnul(sp, '\n');
 		more = (*ep == '\n');
 		*ep = '\0';
 		handle_attr_line(res, sp, path, ++lineno, macro_ok);
-- 
2.11.0.483.g087da7b7c-goog


^ permalink raw reply related

* [PATCH v2 03/27] attr.c: update a stale comment on "struct match_attr"
From: Brandon Williams @ 2017-01-23 20:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, sbeller, pclouds, Brandon Williams
In-Reply-To: <20170123203525.185058-1-bmwill@google.com>

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

When 82dce998 (attr: more matching optimizations from .gitignore,
2012-10-15) changed a pointer to a string "*pattern" into an
embedded "struct pattern" in struct match_attr, it forgot to update
the comment that describes the structure.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
 attr.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/attr.c b/attr.c
index 04d24334e..007f1a299 100644
--- a/attr.c
+++ b/attr.c
@@ -131,9 +131,8 @@ struct pattern {
  * If is_macro is true, then u.attr is a pointer to the git_attr being
  * defined.
  *
- * If is_macro is false, then u.pattern points at the filename pattern
- * to which the rule applies.  (The memory pointed to is part of the
- * memory block allocated for the match_attr instance.)
+ * If is_macro is false, then u.pat is the filename pattern to which the
+ * rule applies.
  *
  * In either case, num_attr is the number of attributes affected by
  * this rule, and state is an array listing them.  The attributes are
-- 
2.11.0.483.g087da7b7c-goog


^ permalink raw reply related

* [PATCH v2 04/27] attr.c: explain the lack of attr-name syntax check in parse_attr()
From: Brandon Williams @ 2017-01-23 20:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, sbeller, pclouds, Brandon Williams
In-Reply-To: <20170123203525.185058-1-bmwill@google.com>

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

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
 attr.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/attr.c b/attr.c
index 007f1a299..6b55a57ef 100644
--- a/attr.c
+++ b/attr.c
@@ -183,6 +183,12 @@ static const char *parse_attr(const char *src, int lineno, const char *cp,
 			return NULL;
 		}
 	} else {
+		/*
+		 * As this function is always called twice, once with
+		 * e == NULL in the first pass and then e != NULL in
+		 * the second pass, no need for invalid_attr_name()
+		 * check here.
+		 */
 		if (*cp == '-' || *cp == '!') {
 			e->setto = (*cp == '-') ? ATTR__FALSE : ATTR__UNSET;
 			cp++;
-- 
2.11.0.483.g087da7b7c-goog


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox