Re: [PATCH] Add shell completion for git remote rm

[Keith Smiley]

I'm definitely happy to update this patch for now to just complete the 
remote names, and not add rm to the list of subcommand completions if 
we're all ok with that!


--
Keith Smiley

On 12/30, Ævar Arnfjörð Bjarmason wrote:


On Sat, Dec 30 2017, Todd Zullinger jotted:


Ævar Arnfjörð Bjarmason wrote:

I think adding 'rm' to completion definitely counts as advertisement.
It doesn't have much practical use, after all: typing 'rm' with
completion is actually one more keystroke than without (rm vs. rm).


This is only one use of the completion interface, maybe you only use it
like that, but not everyone does.

The completion interface has two uses, one is to actually save you
typing, the other is subcommand discovery, and maybe someone who has a
use neither you or I have thought of is about to point out a third.

I'll type "git $whatever $subcommand" as *validation* that I've
found the right command, not to complete it for me. This is a thing
that's in my muscle memory for everything.


Is that meant to be in favor of including rm in the
completion results or against? :)


For.


Since I've been typing "git remote rm" for a while (started before
this deprecation happened) I've actually been meaning to submit
completion for "rm" since it works, not knowing about Duy's patch until
now.

Now, even if someone disagrees that we should have "rm" at all I think
that in general we should not conflate two different things, one is
whether:

git remote 

shows both "rm" and "remove" in the list, and the other is whether:

git remote rm

Should yield:

git remote rm

Or, as now happens:

git remote rm

I can see why we'd, in general, we'd like to not advertise certain
options for completion (due to deprecation, or just to avoid verbosity),
but complete them once they're unambiguously typed out.

I don't know whether the bash completion interface supports making that
distinction, but it would be useful.


It can be done, though I think that it's probably better to
subtly lead people to use 'git remote remove' going forward,
to keep things consistent.  I don't have a strong preference
for or against the rm -> remove change, but since that's
been done I think there's a benefit to keeping things
consistent in the UI.


We changed it in the past, we can always change it again, it's never too
late to fix the UI.

Now whether we *should* change/fix this particular thing is another
matter. I'm just pointing out that we shouldn't fall into the trap of
thinking that git's UI is an established platform that can't be changed.

The vast majority of people who'll ever use git will most likely start
using a version that we're going to release many years into the future.

I'm reminded of the story about the guy who decided makefiles must have
tabs, who didn't want to change it because he already had some dozens of
users.


And I think that should also apply to
not offering completion for commands/subcommands/options
which are only kept for backward compatibility.


Yeah I think it makes sense to at some point stop completing things if
we're going to remove stuff, if we decide to remove it.


Here's one way to make 'git remote rm ' work without
including it in the output of 'git remote ':

diff --git i/contrib/completion/git-completion.bash 
w/contrib/completion/git-completion.bash
index 3683c772c5..aa63f028ab 100644
--- i/contrib/completion/git-completion.bash
+++ w/contrib/completion/git-completion.bash
@@ -2668,7 +2668,9 @@ _git_remote ()
add rename remove set-head set-branches
get-url set-url show prune update
"
-   local subcommand="$(__git_find_on_cmdline "$subcommands")"
+   # Don't advertise rm by including it in subcommands, but complete
+   # remotes if it is used.
+   local subcommand="$(__git_find_on_cmdline "$subcommands rm")"
if [ -z "$subcommand" ]; then
case "$cur" in
--*)


Neat!


Keeping 'git remote rm' working to avoid breaking scripts is
one thing, but having it in the completion code makes it
more likely that it will continue to be seen as a
recommended subcommand.

This leads to patches like this one, where it's presumed
that the lack of completion is simply an oversight or a bug.
Of course, the lack of completion hasn't caused everyone to
forget that 'remote rm' was changed to 'remote remove', so
that reasoning may be full of hot air (or worse). ;)

The current result of 'git remote rm ' isn't so great.
It's arguably worse to have it pretend that no subcommand
was given than to list the remotes.

$ git remote rm 
addremove set-head   update
get-urlrename set-url
prune  set-branches   show


Although that's a bug that has nothing to do with remove/rm, because you
also get:

   $ git remote blahblah 
   $ git rebase doesntexist 

etc. showing you valid subcommands, when perhaps we should show
"warning: no such subcommand `blahblah`/`doesntexist`!" 

[PATCH 2/2] mv: remove unneeded 'if (!show_only)'

[Stefan Moch]

Commit a127331cd (mv: allow moving nested submodules,
2016-04-19), introduced

if (show_only) continue;

in this for-loop before

if (!show_only)

which became redundant, because it is now always true.

Signed-off-by: Stefan Moch 
---
 builtin/mv.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index cf3684d90..8ce6a2ddd 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -286,8 +286,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 
pos = cache_name_pos(src, strlen(src));
assert(pos >= 0);
-   if (!show_only)
-   rename_cache_entry_at(pos, dst);
+   rename_cache_entry_at(pos, dst);
}
 
if (gitmodules_modified)
-- 
2.14.3

[PATCH 1/2] Add test case for mv --dry-run to t7001-mv.sh

[Stefan Moch]

It checks if mv --dry-run does not move file.

Signed-off-by: Stefan Moch 
---
 t/t7001-mv.sh | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 6e5031f56..d4e6485a2 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -38,6 +38,12 @@ test_expect_success \
 'git diff-tree -r -M --name-status  HEAD^ HEAD | \
 grep "^R100..*path1/COPYING..*path0/COPYING"'
 
+test_expect_success \
+'mv --dry-run does not move file' \
+'git mv -n path0/COPYING MOVED &&
+ test -f path0/COPYING &&
+ test ! -f MOVED'
+
 test_expect_success \
 'checking -k on non-existing file' \
 'git mv -k idontexist path0'
-- 
2.14.3

Re: feature-request: git "cp" like there is git mv.

[Stefan Moch]

* Jonathan Nieder  [2017-12-15T17:31:30-0800]:
> This sounds like a reasonable thing to add.  See builtin/mv.c for how
> "git mv" works if you're looking for inspiration.
> 
> cmd_mv in that file looks rather long, so I'd also be happy if someone
> interested refactors to break it into multiple self-contained pieces
> for easier reading (git mostly follows
> https://www.kernel.org/doc/html/latest/process/coding-style.html#functions).

I looked at builtin/mv.c and have a rough idea how to split it
up to support both mv and cp commands.

But first I noticed and removed a redundant check in cmd_mv,
also added a test case to check if mv --dry-run does not move
the file.


Stefan

[PATCH 2/2] travis-ci: check that all build artifacts are .gitignore-d

[SZEDER Gábor]

Every once in a while our explicit .gitignore files get out of sync
when our build process learns to create new artifacts, like test
helper executables, but the .gitignore files are not updated
accordingly.

Use Travis CI to help catch such issues earlier: check that there are
no untracked files at the end of any build jobs building Git (i.e. the
64 bit Clang and GCC Linux and OSX build jobs, plus the GETTEXT_POISON
and 32 bit Linux build jobs) or its documentation, and fail the build
job if there are any present.

Signed-off-by: SZEDER Gábor 
---
 ci/lib-travisci.sh   | 10 ++
 ci/run-linux32-docker.sh |  2 ++
 ci/run-tests.sh  |  2 ++
 ci/test-documentation.sh |  6 ++
 4 files changed, 20 insertions(+)

diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh
index 1543b7959..07f27c727 100755
--- a/ci/lib-travisci.sh
+++ b/ci/lib-travisci.sh
@@ -67,6 +67,16 @@ skip_good_tree () {
exit 0
 }
 
+check_unignored_build_artifacts ()
+{
+   ! git ls-files --other --exclude-standard --error-unmatch \
+   -- ':/*' 2>/dev/null ||
+   {
+   echo "$(tput setaf 1)error: found unignored build 
artifacts$(tput sgr0)"
+   false
+   }
+}
+
 # Set 'exit on error' for all CI scripts to let the caller know that
 # something went wrong.
 # Set tracing executed commands, primarily setting environment variables
diff --git a/ci/run-linux32-docker.sh b/ci/run-linux32-docker.sh
index 870a41246..4f191c5bb 100755
--- a/ci/run-linux32-docker.sh
+++ b/ci/run-linux32-docker.sh
@@ -23,4 +23,6 @@ docker run \
daald/ubuntu32:xenial \
/usr/src/git/ci/run-linux32-build.sh $(id -u $USER)
 
+check_unignored_build_artifacts
+
 save_good_tree
diff --git a/ci/run-tests.sh b/ci/run-tests.sh
index eb5ba4058..22355f009 100755
--- a/ci/run-tests.sh
+++ b/ci/run-tests.sh
@@ -8,4 +8,6 @@
 ln -s $HOME/travis-cache/.prove t/.prove
 make --quiet test
 
+check_unignored_build_artifacts
+
 save_good_tree
diff --git a/ci/test-documentation.sh b/ci/test-documentation.sh
index 3d62e6c95..a20de9ca1 100755
--- a/ci/test-documentation.sh
+++ b/ci/test-documentation.sh
@@ -18,6 +18,9 @@ test -s Documentation/git.xml
 test -s Documentation/git.1
 grep 

[PATCH 0/2] Travis CI: check unignored build artifacts

[SZEDER Gábor]

Every once in a while our explicit .gitignore files get out of sync when
our build process learns to create new artifacts, but the .gitignore
files are not updated accordingly.  It was recently that we got a report
about unignored test helper executables, see 44103f419 (t/helper: ignore
everything but sources, 2017-12-12).

This short patch series teaches our Travis CI build scripts to detect
unignored build artifacts at the end of builds, in the hope to catch
these issues earlier.

These patches should go on top 'sg/travis-skip-identical-test'.  The two
patch series are conceptually independent, but would have a couple of
conflicts when applied separately and then merged together, and I don't
think it's worth carrying them in separate branches.


SZEDER Gábor (2):
  travis-ci: don't store P4 and Git LFS in the working tree
  travis-ci: check that all build artifacts are .gitignore-d

 ci/lib-travisci.sh   | 14 --
 ci/run-linux32-docker.sh |  2 ++
 ci/run-tests.sh  |  2 ++
 ci/test-documentation.sh |  6 ++
 4 files changed, 22 insertions(+), 2 deletions(-)

-- 
2.16.0.rc0.67.g3a46dbca7

[PATCH 1/2] travis-ci: don't store P4 and Git LFS in the working tree

[SZEDER Gábor]

The Clang and GCC 64 bit Linux build jobs download and store the P4
and Git LFS executables under the current directory, which is the
working tree that we are about to build and test.  This means that Git
commands like 'status' or 'ls-files' would list these files as
untracked.  The next commit is about to make sure that there are no
untracked files present after the build, and the downloaded
executables in the working tree are interfering with those upcoming
checks.

Therefore, let's download P4 and Git LFS in the home directory,
outside of the working tree.

Signed-off-by: SZEDER Gábor 
---
 ci/lib-travisci.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh
index bade71617..1543b7959 100755
--- a/ci/lib-travisci.sh
+++ b/ci/lib-travisci.sh
@@ -99,8 +99,8 @@ linux-clang|linux-gcc)
export LINUX_P4_VERSION="16.2"
export LINUX_GIT_LFS_VERSION="1.5.2"
 
-   P4_PATH="$(pwd)/custom/p4"
-   GIT_LFS_PATH="$(pwd)/custom/git-lfs"
+   P4_PATH="$HOME/custom/p4"
+   GIT_LFS_PATH="$HOME/custom/git-lfs"
export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
;;
 osx-clang|osx-gcc)
-- 
2.16.0.rc0.67.g3a46dbca7

[L10N] Kickoff of translation for Git 2.16.0 round 1

[Jiang Xin]

Hi guys,

Happy new year.

New round of Git l10n is coming. It's time to start l10n for Git 2.16.0.
This time there are 64 updated messages need to be translated since last
update:

l10n: git.pot: v2.16.0 round 1 (64 new, 25 removed)

Generate po/git.pot from v2.16.0-rc0 for git v2.16.0 l10n round 1.

Signed-off-by: Jiang Xin 

You can get it from the usual place:

https://github.com/git-l10n/git-po/

As how to update your XX.po and help to translate Git, please see
"Updating a XX.po file" and other sections in “po/README" file.

--
Jiang Xin

Re: [PATCH 1/5] convert_to_git(): checksafe becomes an integer

[Lars Schneider]

> On 31 Dec 2017, at 09:05, tbo...@web.de wrote:
> 
> From: Torsten Bögershausen 
> 
> When calling convert_to_git(), the checksafe parameter has been used to
> check if commit would give a non-roundtrip conversion of EOL.
> 
> When checksafe was introduced, 3 values had been in use:
> SAFE_CRLF_FALSE: no warning
> SAFE_CRLF_FAIL:  reject the commit if EOL do not roundtrip
> SAFE_CRLF_WARN:  warn the user if EOL do not roundtrip

In general, I really like the direction as this simplifies
my patch later on in 5/5. However, I see a few problems:

(1) The prefix "SAFE_CRLF" confuses me because the main theme
is EOL and CRLF just happens to be a EOL type.

What do you think about something like this:
CONVERT_ERROR_IGNORE 0
CONVERT_EOL_ERROR_DIE(1<<0)
CONVERT_EOL_ERROR_WARN   (1<<1)
CONVERT_EOL_TO_LF(1<<2)
CONVERT_EOL_KEEP_CRLF(1<<3)
CONVERT_ENCODE_ERROR_DIE (1<<4)

(2) We mix error reporting switches (FALSE/FAIL/WARN) with
switches that change the content (RENORMALIZE/KEEP_CRLF).
Plus, these the switches should be mutually exclusive
(e.g. we don't want to enable the FAIL and WARN bit at
the same time).

(3) We kind of replicate some flags defined in cache.h:
#define HASH_WRITE_OBJECT 1
#define HASH_RENORMALIZE  4


- Lars

Re: [PATCHv2 0/3] Travis CI: skip commits with successfully built and tested trees

[Lars Schneider]

> On 31 Dec 2017, at 11:12, SZEDER Gábor  wrote:
> 
> This is the second iteration of 'sg/travis-skip-identical-test',
> addressing the comments of Lars and Jonathan:
> 
>  - Colorize the "Tip of $TRAVIS_BRANCH is exactly at $TAG" message
>in the new patch 1/3.
> 
>  - Create the cache directory at the beginning of the build process
>(patch 2/3).
> 
>  - Limit the the cached good trees file size to 1000 records, to
>prevent it from growing too large for git/git's forever living
>integration branches (patch 3/3).
> 
>  - Colorize the first line of the "skip build job because this tree has
>been tested".  Green it is (3/3).
> 
>  - Removed stray whitespace (3/3).
> 
>  - Updated an in-code comment, to make clear which code path deals with
>a non-existing good trees file (3/3).


This series looks good to me.
Nice work!

- Lars

[PATCHv2 0/3] Travis CI: skip commits with successfully built and tested trees

[SZEDER Gábor]

This is the second iteration of 'sg/travis-skip-identical-test',
addressing the comments of Lars and Jonathan:

  - Colorize the "Tip of $TRAVIS_BRANCH is exactly at $TAG" message
in the new patch 1/3.

  - Create the cache directory at the beginning of the build process
(patch 2/3).

  - Limit the the cached good trees file size to 1000 records, to
prevent it from growing too large for git/git's forever living
integration branches (patch 3/3).

  - Colorize the first line of the "skip build job because this tree has
been tested".  Green it is (3/3).

  - Removed stray whitespace (3/3).

  - Updated an in-code comment, to make clear which code path deals with
a non-existing good trees file (3/3).

SZEDER Gábor (3):
  travis-ci: print the "tip of branch is exactly at tag" message in
color
  travis-ci: create the cache directory early in the build process
  travis-ci: record and skip successfully built trees

 ci/lib-travisci.sh| 51 ++-
 ci/run-linux32-docker.sh  |  2 ++
 ci/run-static-analysis.sh |  2 ++
 ci/run-tests.sh   |  3 ++-
 ci/run-windows-build.sh   |  2 ++
 ci/test-documentation.sh  |  2 ++
 6 files changed, 60 insertions(+), 2 deletions(-)

-- 
2.16.0.rc0.67.g3a46dbca7


diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh
index 05e73123f..bade71617 100755
--- a/ci/lib-travisci.sh
+++ b/ci/lib-travisci.sh
@@ -16,7 +16,7 @@ skip_branch_tip_with_tag () {
if TAG=$(git describe --exact-match "$TRAVIS_BRANCH" 2>/dev/null) &&
test "$TAG" != "$TRAVIS_BRANCH"
then
-   echo "Tip of $TRAVIS_BRANCH is exactly at $TAG"
+   echo "$(tput setaf 2)Tip of $TRAVIS_BRANCH is exactly at 
$TAG$(tput sgr0)"
exit 0
fi
 }
@@ -28,6 +28,9 @@ good_trees_file="$HOME/travis-cache/good-trees"
 # message.
 save_good_tree () {
echo "$(git rev-parse $TRAVIS_COMMIT^{tree}) $TRAVIS_COMMIT 
$TRAVIS_JOB_NUMBER $TRAVIS_JOB_ID" >>"$good_trees_file"
+   # limit the file size
+   tail -1000 "$good_trees_file" >"$good_trees_file".tmp
+   mv "$good_trees_file".tmp "$good_trees_file"
 }
 
 # Skip the build job if the same tree has already been built and tested
@@ -36,23 +39,24 @@ save_good_tree () {
 skip_good_tree () {
if ! good_tree_info="$(grep "^$(git rev-parse $TRAVIS_COMMIT^{tree}) " 
"$good_trees_file")"
then
-   # haven't seen this tree yet; continue the build job
+   # Haven't seen this tree yet, or no cached good trees file yet.
+   # Continue the build job.
return
fi
 
echo "$good_tree_info" | {
read tree prev_good_commit prev_good_job_number prev_good_job_id
 
-   if test "$TRAVIS_JOB_ID" =  "$prev_good_job_id"
+   if test "$TRAVIS_JOB_ID" = "$prev_good_job_id"
then
cat <<-EOF
-   Skipping build job for commit $TRAVIS_COMMIT.
+   $(tput setaf 2)Skipping build job for commit 
$TRAVIS_COMMIT.$(tput sgr0)
This commit has already been built and tested 
successfully by this build job.
To force a re-build delete the branch's cache and then 
hit 'Restart job'.
EOF
else
cat <<-EOF
-   Skipping build job for commit $TRAVIS_COMMIT.
+   $(tput setaf 2)Skipping build job for commit 
$TRAVIS_COMMIT.$(tput sgr0)
This commit's tree has already been built and tested 
successfully in build job $prev_good_job_number for commit $prev_good_commit.
The log of that build job is available at 
https://travis-ci.org/$TRAVIS_REPO_SLUG/jobs/$prev_good_job_id
To force a re-build delete the branch's cache and then 
hit 'Restart job'.
@@ -69,6 +73,8 @@ skip_good_tree () {
 # and installing dependencies.
 set -ex
 
+mkdir -p "$HOME/travis-cache"
+
 skip_branch_tip_with_tag
 skip_good_tree
 

[PATCHv2 3/3] travis-ci: record and skip successfully built trees

[SZEDER Gábor]

Travis CI dutifully builds and tests each new branch tip, even if its
tree has previously been successfully built and tested.  This happens
often enough in contributors' workflows, when a work-in-progress
branch is rebased changing e.g. only commit messages or the order or
number of commits while leaving the resulting code intact, and is then
pushed to a Travis CI-enabled GitHub fork.

This is wasting Travis CI's resources and is sometimes scary-annoying
when the new tip commit with a tree identical to the previous,
successfully tested one is suddenly reported in red, because one of
the OSX build jobs happened to exceed the time limit yet again.

So extend our Travis CI build scripts to skip building commits whose
trees have previously been successfully built and tested.  Use the
Travis CI cache feature to keep a record of the object names of trees
that tested successfully, in a plain and simple flat text file, one
line per tree object name.  Append the current tree's object name at
the end of every successful build job to this file, along with a bit
of additional info about the build job (commit object name, Travis CI
job number and id).  Limit the size of this file to 1000 records, to
prevent it from growing too large for git/git's forever living
integration branches.  Check, using a simple grep invocation, in each
build job whether the current commit's tree is already in there, and
skip the build if it is.  Include a message in the skipped build job's
trace log, containing the URL to the build job successfully testing
that tree for the first time and instructions on how to force a
re-build.  Catch the case when a build job, which successfully built
and tested a particular tree for the first time, is restarted and omit
the URL of the previous build job's trace log, as in this case it's
the same build job and the trace log has just been overwritten.

Note: this won't kick in if two identical trees are on two different
branches, because Travis CI caches are not shared between build jobs
of different branches.

Signed-off-by: SZEDER Gábor 
---
 ci/lib-travisci.sh| 47 +++
 ci/run-linux32-docker.sh  |  2 ++
 ci/run-static-analysis.sh |  2 ++
 ci/run-tests.sh   |  2 ++
 ci/run-windows-build.sh   |  2 ++
 ci/test-documentation.sh  |  2 ++
 6 files changed, 57 insertions(+)

diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh
index 197aa14c1..bade71617 100755
--- a/ci/lib-travisci.sh
+++ b/ci/lib-travisci.sh
@@ -21,6 +21,52 @@ skip_branch_tip_with_tag () {
fi
 }
 
+good_trees_file="$HOME/travis-cache/good-trees"
+
+# Save some info about the current commit's tree, so we can skip the build
+# job if we encounter the same tree again and can provide a useful info
+# message.
+save_good_tree () {
+   echo "$(git rev-parse $TRAVIS_COMMIT^{tree}) $TRAVIS_COMMIT 
$TRAVIS_JOB_NUMBER $TRAVIS_JOB_ID" >>"$good_trees_file"
+   # limit the file size
+   tail -1000 "$good_trees_file" >"$good_trees_file".tmp
+   mv "$good_trees_file".tmp "$good_trees_file"
+}
+
+# Skip the build job if the same tree has already been built and tested
+# successfully before (e.g. because the branch got rebased, changing only
+# the commit messages).
+skip_good_tree () {
+   if ! good_tree_info="$(grep "^$(git rev-parse $TRAVIS_COMMIT^{tree}) " 
"$good_trees_file")"
+   then
+   # Haven't seen this tree yet, or no cached good trees file yet.
+   # Continue the build job.
+   return
+   fi
+
+   echo "$good_tree_info" | {
+   read tree prev_good_commit prev_good_job_number prev_good_job_id
+
+   if test "$TRAVIS_JOB_ID" = "$prev_good_job_id"
+   then
+   cat <<-EOF
+   $(tput setaf 2)Skipping build job for commit 
$TRAVIS_COMMIT.$(tput sgr0)
+   This commit has already been built and tested 
successfully by this build job.
+   To force a re-build delete the branch's cache and then 
hit 'Restart job'.
+   EOF
+   else
+   cat <<-EOF
+   $(tput setaf 2)Skipping build job for commit 
$TRAVIS_COMMIT.$(tput sgr0)
+   This commit's tree has already been built and tested 
successfully in build job $prev_good_job_number for commit $prev_good_commit.
+   The log of that build job is available at 
https://travis-ci.org/$TRAVIS_REPO_SLUG/jobs/$prev_good_job_id
+   To force a re-build delete the branch's cache and then 
hit 'Restart job'.
+   EOF
+   fi
+   }
+
+   exit 0
+}
+
 # Set 'exit on error' for all CI scripts to let the caller know that
 # something went wrong.
 # Set tracing executed commands, primarily setting environment variables
@@ -30,6 +76,7 @@ set -ex
 mkdir -p "$HOME/travis-cache"
 
 

[PATCHv2 2/3] travis-ci: create the cache directory early in the build process

[SZEDER Gábor]

It seems that Travis CI creates the cache directory for us anyway,
even when a previous cache doesn't exist for the current build job.
Alas, this behavior is not explicitly documented, therefore we don't
rely on it and create the cache directory ourselves in those build
jobs that read/write cached data (currently only the prove state).

In the following commit we'll start to cache additional data in every
build job, and will access the cache much earlier in the build
process.

Therefore move creating the cache directory to 'ci/lib-travisci.sh' to
make sure that it exists at the very beginning of every build job.

Signed-off-by: SZEDER Gábor 
---
 ci/lib-travisci.sh | 2 ++
 ci/run-tests.sh| 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh
index 9d379db8a..197aa14c1 100755
--- a/ci/lib-travisci.sh
+++ b/ci/lib-travisci.sh
@@ -27,6 +27,8 @@ skip_branch_tip_with_tag () {
 # and installing dependencies.
 set -ex
 
+mkdir -p "$HOME/travis-cache"
+
 skip_branch_tip_with_tag
 
 if test -z "$jobname"
diff --git a/ci/run-tests.sh b/ci/run-tests.sh
index f0c743de9..ccdfc2b9d 100755
--- a/ci/run-tests.sh
+++ b/ci/run-tests.sh
@@ -5,6 +5,5 @@
 
 . ${0%/*}/lib-travisci.sh
 
-mkdir -p $HOME/travis-cache
 ln -s $HOME/travis-cache/.prove t/.prove
 make --quiet test
-- 
2.16.0.rc0.67.g3a46dbca7

[PATCHv2 1/3] travis-ci: print the "tip of branch is exactly at tag" message in color

[SZEDER Gábor]

To make this info message stand out from the regular build job trace
output.

Signed-off-by: SZEDER Gábor 
---
 ci/lib-travisci.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh
index 348fe3c3c..9d379db8a 100755
--- a/ci/lib-travisci.sh
+++ b/ci/lib-travisci.sh
@@ -16,7 +16,7 @@ skip_branch_tip_with_tag () {
if TAG=$(git describe --exact-match "$TRAVIS_BRANCH" 2>/dev/null) &&
test "$TAG" != "$TRAVIS_BRANCH"
then
-   echo "Tip of $TRAVIS_BRANCH is exactly at $TAG"
+   echo "$(tput setaf 2)Tip of $TRAVIS_BRANCH is exactly at 
$TAG$(tput sgr0)"
exit 0
fi
 }
-- 
2.16.0.rc0.67.g3a46dbca7

Re: Segfault

[Eric Sunshine]

On Fri, Dec 29, 2017 at 4:04 AM, Andrew Tsykhonya
 wrote:
> git stash pop resulted in crash:
> /usr/local/Cellar/git/2.10.2/libexec/git-core/git-stash: line 470:
> 14946 Segmentation fault: 11 git merge-recursive $b_tree -- $c_tree
> $w_tree
> although, the changes have been applied successfully.

Thanks for the problem report. Can you come up with a reproduction
recipe in order to help debug the problem?

Also, what happens if you update to a newer version of Git? You appear
to be running 2.10.2, whereas the latest version in Homebrew seems to
be 2.15.1.

[PATCH 2/5] strbuf: add xstrdup_toupper()

[tboegi]

From: Lars Schneider 

Create a copy of an existing string and make all characters upper case.
Similar xstrdup_tolower().

This function is used in a subsequent commit.

Signed-off-by: Lars Schneider 
Signed-off-by: Torsten Bögershausen 
---
 strbuf.c | 13 +
 strbuf.h |  1 +
 2 files changed, 14 insertions(+)

diff --git a/strbuf.c b/strbuf.c
index 8007be8fba..ee05626dc1 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -785,6 +785,19 @@ char *xstrdup_tolower(const char *string)
return result;
 }
 
+char *xstrdup_toupper(const char *string)
+{
+   char *result;
+   size_t len, i;
+
+   len = strlen(string);
+   result = xmallocz(len);
+   for (i = 0; i < len; i++)
+   result[i] = toupper(string[i]);
+   result[i] = '\0';
+   return result;
+}
+
 char *xstrvfmt(const char *fmt, va_list ap)
 {
struct strbuf buf = STRBUF_INIT;
diff --git a/strbuf.h b/strbuf.h
index 14c8c10d66..df7ced53ed 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -607,6 +607,7 @@ __attribute__((format (printf,2,3)))
 extern int fprintf_ln(FILE *fp, const char *fmt, ...);
 
 char *xstrdup_tolower(const char *);
+char *xstrdup_toupper(const char *);
 
 /**
  * Create a newly allocated string using printf format. You can do this easily
-- 
2.16.0.rc0.4.ga4e00d4fa4

[PATCH 4/5] utf8: add function to detect a missing UTF-16/32 BOM

[tboegi]

From: Lars Schneider 

If the endianness is not defined in the encoding name, then let's
be strict and require a BOM to avoid any encoding confusion. The
has_missing_utf_bom() function returns true if a required BOM is
missing.

The Unicode standard instructs to assume big-endian if there in no BOM
for UTF-16/32 [1][2]. However, the W3C/WHATWG encoding standard used
in HTML5 recommends to assume little-endian to "deal with deployed
content" [3]. Strictly requiring a BOM seems to be the safest option
for content in Git.

This function is used in a subsequent commit.

[1] http://unicode.org/faq/utf_bom.html#gen6
[2] http://www.unicode.org/versions/Unicode10.0.0/ch03.pdf
 Section 3.10, D98, page 132
[3] https://encoding.spec.whatwg.org/#utf-16le

Signed-off-by: Lars Schneider 
Signed-off-by: Torsten Bögershausen 
---
 utf8.c | 13 +
 utf8.h | 16 
 2 files changed, 29 insertions(+)

diff --git a/utf8.c b/utf8.c
index 776660ee12..1978d6c42a 100644
--- a/utf8.c
+++ b/utf8.c
@@ -562,6 +562,19 @@ int has_prohibited_utf_bom(const char *enc, const char 
*data, size_t len)
);
 }
 
+int has_missing_utf_bom(const char *enc, const char *data, size_t len)
+{
+   return (
+  !strcmp(enc, "UTF-16") &&
+  !(has_bom_prefix(data, len, utf16_be_bom, sizeof(utf16_be_bom)) ||
+has_bom_prefix(data, len, utf16_le_bom, sizeof(utf16_le_bom)))
+   ) || (
+  !strcmp(enc, "UTF-32") &&
+  !(has_bom_prefix(data, len, utf32_be_bom, sizeof(utf32_be_bom)) ||
+has_bom_prefix(data, len, utf32_le_bom, sizeof(utf32_le_bom)))
+   );
+}
+
 /*
  * Returns first character length in bytes for multi-byte `text` according to
  * `encoding`.
diff --git a/utf8.h b/utf8.h
index 4711429af9..26b5e91852 100644
--- a/utf8.h
+++ b/utf8.h
@@ -79,4 +79,20 @@ void strbuf_utf8_align(struct strbuf *buf, align_type 
position, unsigned int wid
  */
 int has_prohibited_utf_bom(const char *enc, const char *data, size_t len);
 
+/*
+ * If the endianness is not defined in the encoding name, then we
+ * require a BOM. The function returns true if a required BOM is missing.
+ *
+ * The Unicode standard instructs to assume big-endian if there
+ * in no BOM for UTF-16/32 [1][2]. However, the W3C/WHATWG
+ * encoding standard used in HTML5 recommends to assume
+ * little-endian to "deal with deployed content" [3].
+ *
+ * [1] http://unicode.org/faq/utf_bom.html#gen6
+ * [2] http://www.unicode.org/versions/Unicode10.0.0/ch03.pdf
+ * Section 3.10, D98, page 132
+ * [3] https://encoding.spec.whatwg.org/#utf-16le
+ */
+int has_missing_utf_bom(const char *enc, const char *data, size_t len);
+
 #endif
-- 
2.16.0.rc0.4.ga4e00d4fa4

[PATCH 0/5] V2B: simplify convert.c/h

[tboegi]

From: Torsten Bögershausen 

Simplify the convert.h/convert.c logic amd don't touch convert_to_git()
The rest is v2 from Lars

Lars Schneider (4):
  strbuf: add xstrdup_toupper()
  utf8: add function to detect prohibited UTF-16/32 BOM
  utf8: add function to detect a missing UTF-16/32 BOM
  convert: add support for 'checkout-encoding' attribute

Torsten Bögershausen (1):
  convert_to_git(): checksafe becomes an integer

 Documentation/gitattributes.txt |  59 +++
 apply.c |   4 +-
 convert.c   | 210 +---
 convert.h   |  19 ++--
 diff.c  |   4 +-
 environment.c   |   2 +-
 sha1_file.c |   8 +-
 strbuf.c|  13 +++
 strbuf.h|   1 +
 t/t0028-checkout-encoding.sh| 197 +
 utf8.c  |  37 +++
 utf8.h  |  25 +
 12 files changed, 549 insertions(+), 30 deletions(-)
 create mode 100755 t/t0028-checkout-encoding.sh

-- 
2.16.0.rc0.4.ga4e00d4fa4

[PATCH 3/5] utf8: add function to detect prohibited UTF-16/32 BOM

[tboegi]

From: Lars Schneider 

Whenever a data stream is declared to be UTF-16BE, UTF-16LE, UTF-32BE
or UTF-32LE a BOM must not be used [1]. The function returns true if
this is the case.

This function is used in a subsequent commit.

[1] http://unicode.org/faq/utf_bom.html#bom10

Signed-off-by: Lars Schneider 
Signed-off-by: Torsten Bögershausen 
---
 utf8.c | 24 
 utf8.h |  9 +
 2 files changed, 33 insertions(+)

diff --git a/utf8.c b/utf8.c
index 2c27ce0137..776660ee12 100644
--- a/utf8.c
+++ b/utf8.c
@@ -538,6 +538,30 @@ char *reencode_string_len(const char *in, int insz,
 }
 #endif
 
+static int has_bom_prefix(const char *data, size_t len,
+ const char *bom, size_t bom_len)
+{
+   return (len >= bom_len) && !memcmp(data, bom, bom_len);
+}
+
+const char utf16_be_bom[] = {0xFE, 0xFF};
+const char utf16_le_bom[] = {0xFF, 0xFE};
+const char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};
+const char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};
+
+int has_prohibited_utf_bom(const char *enc, const char *data, size_t len)
+{
+   return (
+ (!strcmp(enc, "UTF-16BE") || !strcmp(enc, "UTF-16LE")) &&
+ (has_bom_prefix(data, len, utf16_be_bom, sizeof(utf16_be_bom)) ||
+  has_bom_prefix(data, len, utf16_le_bom, sizeof(utf16_le_bom)))
+   ) || (
+ (!strcmp(enc, "UTF-32BE") || !strcmp(enc, "UTF-32LE")) &&
+ (has_bom_prefix(data, len, utf32_be_bom, sizeof(utf32_be_bom)) ||
+  has_bom_prefix(data, len, utf32_le_bom, sizeof(utf32_le_bom)))
+   );
+}
+
 /*
  * Returns first character length in bytes for multi-byte `text` according to
  * `encoding`.
diff --git a/utf8.h b/utf8.h
index 6bbcf31a83..4711429af9 100644
--- a/utf8.h
+++ b/utf8.h
@@ -70,4 +70,13 @@ typedef enum {
 void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int 
width,
   const char *s);
 
+/*
+ * Whenever a data stream is declared to be UTF-16BE, UTF-16LE, UTF-32BE
+ * or UTF-32LE a BOM must not be used [1]. The function returns true if
+ * this is the case.
+ *
+ * [1] http://unicode.org/faq/utf_bom.html#bom10
+ */
+int has_prohibited_utf_bom(const char *enc, const char *data, size_t len);
+
 #endif
-- 
2.16.0.rc0.4.ga4e00d4fa4

[PATCH 5/5] convert: add support for 'checkout-encoding' attribute

[tboegi]

From: Lars Schneider 

Git and its tools (e.g. git diff) expect all text files in UTF-8
encoding. Git will happily accept content in all other encodings, too,
but it might not be able to process the text (e.g. viewing diffs or
changing line endings).

Add an attribute to tell Git what encoding the user has defined for a
given file. If the content is added to the index, then Git converts the
content to a canonical UTF-8 representation. On checkout Git will
reverse the conversion.

Signed-off-by: Lars Schneider 
Signed-off-by: Torsten Bögershausen 
---
 Documentation/gitattributes.txt |  59 
 convert.c   | 190 +-
 convert.h   |  11 ++-
 sha1_file.c |   2 +-
 t/t0028-checkout-encoding.sh| 197 
 5 files changed, 452 insertions(+), 7 deletions(-)
 create mode 100755 t/t0028-checkout-encoding.sh

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 30687de81a..0039bd38c3 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -272,6 +272,65 @@ few exceptions.  Even though...
   catch potential problems early, safety triggers.
 
 
+`checkout-encoding`
+^^^
+
+Git recognizes files encoded with ASCII or one of its supersets (e.g.
+UTF-8 or ISO-8859-1) as text files.  All other encodings are usually
+interpreted as binary and consequently built-in Git text processing
+tools (e.g. 'git diff') as well as most Git web front ends do not
+visualize the content.
+
+In these cases you can teach Git the encoding of a file in the working
+directory with the `checkout-encoding` attribute. If a file with this
+attributes is added to Git, then Git reencodes the content from the
+specified encoding to UTF-8 and stores the result in its internal data
+structure. On checkout the content is encoded back to the specified
+encoding.
+
+Please note that using the `checkout-encoding` attribute has a number
+of drawbacks:
+
+- Reencoding content to non-UTF encodings (e.g. SHIFT-JIS) can cause
+  errors as the conversion might not be round trip safe.
+
+- Reencoding content requires resources that might slow down certain
+  Git operations (e.g 'git checkout' or 'git add').
+
+- Git clients that do not support the `checkout-encoding` attribute or
+  the used encoding will checkout the respective files as UTF-8 encoded.
+  That means the content appears to be different which could cause
+  trouble. Affected clients are older Git versions and alternative Git
+  implementations such as JGit or libgit2 (as of January 2018).
+
+Use the `checkout-encoding` attribute only if you cannot store a file in
+UTF-8 encoding and if you want Git to be able to process the content as
+text.
+
+Use the following attributes if your '*.txt' files are UTF-16 encoded
+with byte order mark (BOM) and you want Git to perform automatic line
+ending conversion based on your platform.
+
+
+*.txt  text checkout-encoding=UTF-16
+
+
+Use the following attributes if your '*.txt' files are UTF-16 little
+endian encoded without BOM and you want Git to use Windows line endings
+in the working directory.
+
+
+*.txt  checkout-encoding=UTF-16LE text eol=CRLF
+
+
+You can get a list of all available encodings on your platform with the
+following command:
+
+
+iconv --list
+
+
+
 `ident`
 ^^^
 
diff --git a/convert.c b/convert.c
index 5efcc3b73b..22c70d87e5 100644
--- a/convert.c
+++ b/convert.c
@@ -7,6 +7,7 @@
 #include "sigchain.h"
 #include "pkt-line.h"
 #include "sub-process.h"
+#include "utf8.h"
 
 /*
  * convert.c - convert a file when checking it out and checking it in.
@@ -265,6 +266,147 @@ static int will_convert_lf_to_crlf(size_t len, struct 
text_stat *stats,
 
 }
 
+static struct encoding {
+   const char *name;
+   struct encoding *next;
+} *encoding, **encoding_tail;
+static const char *default_encoding = "UTF-8";
+
+static int encode_to_git(const char *path, const char *src, size_t src_len,
+struct strbuf *buf, struct encoding *enc, int 
die_on_failure)
+{
+   char *dst;
+   int dst_len;
+
+   /*
+* No encoding is specified or there is nothing to encode.
+* Tell the caller that the content was not modified.
+*/
+   if (!enc || (src && !src_len))
+   return 0;
+
+   /*
+* Looks like we got called from "would_convert_to_git()".
+* This means Git wants to know if it would encode (= modify!)
+* the content. Let's answer with "yes", since an encoding was
+* specified.
+*/
+   if (!buf && !src)
+   return 1;
+
+   if (has_prohibited_utf_bom(enc->name, src, src_len)) {
+   

[PATCH 1/5] convert_to_git(): checksafe becomes an integer

[tboegi]

From: Torsten Bögershausen 

When calling convert_to_git(), the checksafe parameter has been used to
check if commit would give a non-roundtrip conversion of EOL.

When checksafe was introduced, 3 values had been in use:
SAFE_CRLF_FALSE: no warning
SAFE_CRLF_FAIL:  reject the commit if EOL do not roundtrip
SAFE_CRLF_WARN:  warn the user if EOL do not roundtrip

Today a small flaw is found in the code base:
An integer with the value 0 is passed as the parameter checksafe
instead of the correct enum value SAFE_CRLF_FALSE.

In the next commit there is a need to turn checksafe into a bitmap, which
allows to tell convert_to_git() to obey the encoding attribute or not.

Signed-off-by: Torsten Bögershausen 
---
 apply.c   |  4 ++--
 convert.c | 20 ++--
 convert.h | 18 --
 diff.c|  4 ++--
 environment.c |  2 +-
 sha1_file.c   |  6 +++---
 6 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/apply.c b/apply.c
index 321a9fa68d..a422516062 100644
--- a/apply.c
+++ b/apply.c
@@ -2263,7 +2263,7 @@ static void show_stats(struct apply_state *state, struct 
patch *patch)
 static int read_old_data(struct stat *st, struct patch *patch,
 const char *path, struct strbuf *buf)
 {
-   enum safe_crlf safe_crlf = patch->crlf_in_old ?
+   int checksafe = patch->crlf_in_old ?
SAFE_CRLF_KEEP_CRLF : SAFE_CRLF_RENORMALIZE;
switch (st->st_mode & S_IFMT) {
case S_IFLNK:
@@ -2281,7 +2281,7 @@ static int read_old_data(struct stat *st, struct patch 
*patch,
 * should never look at the index when explicit crlf option
 * is given.
 */
-   convert_to_git(NULL, path, buf->buf, buf->len, buf, safe_crlf);
+   convert_to_git(NULL, path, buf->buf, buf->len, buf, checksafe);
return 0;
default:
return -1;
diff --git a/convert.c b/convert.c
index 1a41a48e15..5efcc3b73b 100644
--- a/convert.c
+++ b/convert.c
@@ -195,13 +195,13 @@ static enum eol output_eol(enum crlf_action crlf_action)
 
 static void check_safe_crlf(const char *path, enum crlf_action crlf_action,
struct text_stat *old_stats, struct text_stat 
*new_stats,
-   enum safe_crlf checksafe)
+   int checksafe)
 {
if (old_stats->crlf && !new_stats->crlf ) {
/*
 * CRLFs would not be restored by checkout
 */
-   if (checksafe == SAFE_CRLF_WARN)
+   if (checksafe & SAFE_CRLF_WARN)
warning(_("CRLF will be replaced by LF in %s.\n"
  "The file will have its original line"
  " endings in your working directory."), path);
@@ -211,7 +211,7 @@ static void check_safe_crlf(const char *path, enum 
crlf_action crlf_action,
/*
 * CRLFs would be added by checkout
 */
-   if (checksafe == SAFE_CRLF_WARN)
+   if (checksafe & SAFE_CRLF_WARN)
warning(_("LF will be replaced by CRLF in %s.\n"
  "The file will have its original line"
  " endings in your working directory."), path);
@@ -268,7 +268,7 @@ static int will_convert_lf_to_crlf(size_t len, struct 
text_stat *stats,
 static int crlf_to_git(const struct index_state *istate,
   const char *path, const char *src, size_t len,
   struct strbuf *buf,
-  enum crlf_action crlf_action, enum safe_crlf checksafe)
+  enum crlf_action crlf_action, int checksafe)
 {
struct text_stat stats;
char *dst;
@@ -298,12 +298,12 @@ static int crlf_to_git(const struct index_state *istate,
 * unless we want to renormalize in a merge or
 * cherry-pick.
 */
-   if ((checksafe != SAFE_CRLF_RENORMALIZE) &&
+   if ((!(checksafe & SAFE_CRLF_RENORMALIZE)) &&
has_crlf_in_index(istate, path))
convert_crlf_into_lf = 0;
}
-   if ((checksafe == SAFE_CRLF_WARN ||
-   (checksafe == SAFE_CRLF_FAIL)) && len) {
+   if (((checksafe & SAFE_CRLF_WARN) ||
+((checksafe & SAFE_CRLF_FAIL) && len))) {
struct text_stat new_stats;
memcpy(_stats, , sizeof(new_stats));
/* simulate "git add" */
@@ -1129,7 +1129,7 @@ const char *get_convert_attr_ascii(const char *path)
 
 int convert_to_git(const struct index_state *istate,
   const char *path, const char *src, size_t len,
-   struct strbuf *dst, enum safe_crlf checksafe)
+  struct strbuf *dst, int checksafe)
 {
int ret = 0;
struct conv_attrs ca;
@@