Re: Standardization of messages - dot after the sentence

[Michael J Gruber]

sigo venit, vidit, dixit 05.09.2015 14:22:
> I've found really "little bug" with dots in the git output.
> 
> $ git push
> Everything up-to-date
> 
> git pull
> Already up-to-date.
> 
> Could all phrases contain dots? :)
> 

In this case, also both messages mean the same but are phrased
differently, which is suboptimal in more than one way.

It would be good to change them systematically (not just in these 2
spots) - and that would make for a perfect first foray into submitting
patches, hint hint hint :)

Michael
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs

[Junio C Hamano]

Karthik Nayak  writes:

> diff --git a/Documentation/git-for-each-ref.txt 
> b/Documentation/git-for-each-ref.txt
> index d039f40..c5154bb 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -128,13 +128,14 @@ color::
>   are described in `color.branch.*`.
>  
>  align::
> - Left-, middle-, or right-align the content between %(align:..)
> + Left-, middle-, or right-align the content between %(align:...)
>   and %(end). Followed by `:,`, where the
>   `` is either left, right or middle and `` is
>   the total length of the content with alignment. If the
>   contents length is more than the width then no alignment is
>   performed. If used with '--quote' everything in between
> - %(align:..)  and %(end) is quoted.
> + %(align:...) and %(end) is quoted, but if nested then only the
> + topmost level performs quoting.

I am not sure if the description of quote belongs to "align".  Isn't
the general rule at the endgame when there are more opening things
that would buffer its effect up to the corresponding %(end):

 - Some atoms like %(align) and %(if) always require a matching
   %(end).  We call them "opening atoms" and sometimes denote them
   as %($open).

 - When a scripting language specific quoting is in effect, except
   for opening atoms, replacement from every %(atom) is quoted when
   and only when it appears at the top-level (that is, when it
   appears outside %($open)...%(end)).

 - When a scripting language specific quoting is in effect,
   everything between a top-level opening atom and its matching
   %(end) is evaluated according to the semantics of the opening
   atom and its result is quoted.

To put the third item above in a different way, a matching
%($open)...  %(end) pair behaves as if it is a single normal atom,
from the point of view of the quoting rule.  All top-level atoms are
invidivually quoted, whether they are normal atoms or open/end pairs.
And that rule is not limited to %(align).

I am flexible with the terminology, but the point is that I think
the quoting rules are better be specified _outside_ the description
of a particular atom, but as a general rule.

> diff --git a/builtin/tag.c b/builtin/tag.c
> index 9fa1400..f55dfda 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -43,8 +43,8 @@ static int list_tags(struct ref_filter *filter, struct 
> ref_sorting *sorting, con
>  
>   if (!format) {
>   if (filter->lines)
> - format = to_free = 
> xstrfmt("%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)",
> -filter->lines);
> + format = to_free = 
> xstrfmt("%%(align:15,left)%%(refname:short)%%(end) "
> +"%%(contents:lines=%d)", 
> filter->lines);

This line still looks overlong.  Would it help to stop spelling this
as a double "a = b = overlong expression" assignment?

>   /*
>* Perform quote formatting when the stack element is that of
> -  * a modifier atom and right above the first stack element.
> +  * a supporting atom. If nested then perform quote formatting
> +  * only on the topmost supporting atom.
>*/
>   if (!state->stack->prev->prev) {
>   quote_formatting(, current->output.buf, state->quote_style);
> @@ -261,6 +262,22 @@ static void end_atom_handler(struct atom_value *atomv, 
> struct ref_formatting_sta
>   pop_stack_element(>stack);
>  }
>  
> +static int match_atom_name(const char *name, const char *atom_name, const 
> char **val)
> +{
> + const char *body;
> +
> + if (!skip_prefix(name, atom_name, ))
> + return 0; /* doesn't even begin with "atom_name" */
> + if (!body[0] || !body[1]) {
> + *val = NULL; /* %(atom_name) and no customization */
> + return 1;
> + }

This if condition is puzzling.  !body[0] would mean the name was
exactly atom_name, which is what you want to catch.

But the other part does not make any sense to me.  what does
(body[0] != '\0' && !body[1]) mean?  atom_name asked for was "align"
and name was "aligna"?  That is not "%(align) and no customization".

> + if (body[0] != ':')
> + return 0; /* "atom_namefoo" is not "atom_name" or 
> "atom_name:..." */

This one makes sense to me.

> + *val = body + 1; /* "atomname:val" */

Spell that "atom_name:val" perhaps?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs

[Matthieu Moy]

Junio C Hamano  writes:

> I am flexible with the terminology, but the point is that I think
> the quoting rules are better be specified _outside_ the description
> of a particular atom, but as a general rule.

I agree, but for now we have only one %($open) ... %(end) pair, so it
makes sense to me to leave the series as-is and to move some of the
explanations in a general section in the next series (that will
introduce %(if) or another opening atom).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 0/5] git-p4: add support for large file systems

[larsxschneider]

From: Lars Schneider 

Diff to v2:
* add proper commit messages
* split commits into generic large file system and GitLFS implementation
* improve docs, mention clean/smduge filter and add example for clean checkout
* fix spelling
* add option to push large files to server
* use ValueError for gitConfigInt exception
* assert required functions for large file system classes
* rename LFS to GitLFS
* add Python docs for GitLFS class
* move generic code from GitLFS class to git-p4 main code
* make variable that keeps large files for .gitattributes creation a set() to 
avoid duplicated entries
* sort large files in .gitattirubutes to minimize diffs between commits

Thanks to Luke and Junio for feedback!

One thing I don't like about the current implementation is that I don't see a
way to test the "git-p4.pushLargeFiles" config. I could start a git lfs server
locally but that seems a bit too much, no?

Thanks,
Lars

Lars Schneider (5):
  git-p4: add optional type specifier to gitConfig reader
  git-p4: add gitConfigInt reader
  git-p4: return an empty list if a list config has no values
  git-p4: add support for large file systems
  git-p4: add Git LFS backend for large file system

 Documentation/git-p4.txt |  28 +
 git-p4.py| 187 ++---
 t/t9823-git-p4-lfs.sh| 263 +++
 3 files changed, 465 insertions(+), 13 deletions(-)
 create mode 100755 t/t9823-git-p4-lfs.sh

--
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 4/5] git-p4: add support for large file systems

[larsxschneider]

From: Lars Schneider 

Perforce repositories can contain large (binary) files. Migrating these
repositories to Git generates very large local clones. External storage
systems such as Git LFS [1], Git Fat [2], or Git Media [2] try to
address this problem.

Add a generic mechanism to detect large files based on extension,
uncompressed size, and/or compressed size.

[1] https://git-lfs.github.com/
[2] https://github.com/jedbrown/git-fat
[3] https://github.com/alebedev/git-media

Signed-off-by: Lars Schneider 
---
 Documentation/git-p4.txt |  26 +++
 git-p4.py| 114 +++
 2 files changed, 131 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 82aa5d6..e0d0239 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -510,6 +510,32 @@ git-p4.useClientSpec::
option '--use-client-spec'.  See the "CLIENT SPEC" section above.
This variable is a boolean, not the name of a p4 client.
 
+git-p4.largeFileSystem::
+   Specify the system that is used used for large (binary) files.
+   Please note that most of these large file systems depend on the
+   Git clean/smudge filters. These filters are not applied through
+   git-p4. You need to create a fresh clone of the repository after
+   running git-p4.
+
+git-p4.largeFileExtensions::
+   All files matching a file extension in the list will be processed
+   by the large file system. Do not prefix the extensions with '.'.
+
+git-p4.largeFileThreshold::
+   All files with an uncompressed size exceeding the threshold will be
+   processed by the large file system. By default the threshold is
+   defined in bytes. Add the suffix k, m, or g to change the unit.
+
+git-p4.largeFileCompressedThreshold::
+   All files with a compressed size exceeding the threshold will be
+   processed by the large file system. This option might significantly
+   slow down your clone/sync process. By default the threshold is
+   defined in bytes. Add the suffix k, m, or g to change the unit.
+
+git-p4.pushLargeFiles::
+   Boolean variable which defines if large files are automatically
+   pushed to a server.
+
 Submit variables
 
 git-p4.detectRenames::
diff --git a/git-p4.py b/git-p4.py
index 90d3b90..06651a8 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -22,6 +22,8 @@ import platform
 import re
 import shutil
 import stat
+import zipfile
+import zlib
 
 try:
 from subprocess import CalledProcessError
@@ -922,6 +924,17 @@ def wildcard_present(path):
 m = re.search("[*#@%]", path)
 return m is not None
 
+def largeFileSystem():
+try:
+largeFileSystem = getattr(sys.modules[__name__], 
gitConfig('git-p4.largeFileSystem'))
+assert(hasattr(largeFileSystem, "attributeDescription"))
+assert(hasattr(largeFileSystem, "attributeFilter"))
+assert(hasattr(largeFileSystem, "generatePointer"))
+assert(hasattr(largeFileSystem, "pushFile"))
+return largeFileSystem
+except AttributeError as e:
+die('Large file system not supported: %s' % 
gitConfig('git-p4.largeFileSystem'))
+
 class Command:
 def __init__(self):
 self.usage = "usage: %prog [options]"
@@ -2038,6 +2051,7 @@ class P4Sync(Command, P4UserMap):
 self.clientSpecDirs = None
 self.tempBranches = []
 self.tempBranchLocation = "git-p4-tmp"
+self.largeFiles = set()
 
 if gitConfig("git-p4.syncFromOrigin") == "false":
 self.syncWithOrigin = False
@@ -2158,6 +2172,85 @@ class P4Sync(Command, P4UserMap):
 
 return branches
 
+def writeToGitStream(self, gitMode, relPath, contents):
+self.gitStream.write('M %s inline %s\n' % (gitMode, relPath))
+self.gitStream.write('data %d\n' % sum(len(d) for d in contents))
+for d in contents:
+self.gitStream.write(d)
+self.gitStream.write('\n')
+
+def writeGitAttributesToStream(self):
+self.writeToGitStream(
+'100644',
+'.gitattributes',
+[
+'\n',
+'#\n',
+'# %s\n' % largeFileSystem().attributeDescription(),
+'#\n',
+] +
+['*.' + f.replace(' ', '[:space:]') + ' filter=%s -text\n'
+% largeFileSystem().attributeFilter()
+for f in sorted(gitConfigList("git-p4.largeFileExtensions"))
+] +
+['/' + f.replace(' ', '[:space:]') + ' filter=%s -text\n'
+% largeFileSystem().attributeFilter()
+for f in sorted(self.largeFiles) if not 
self.hasLargeFileExtension(f)
+]
+)
+
+def hasLargeFileExtension(self, relPath):
+return reduce(
+lambda a, b: a or b,
+[relPath.endswith('.' + e) for e in 

[PATCH v3 5/5] git-p4: add Git LFS backend for large file system

[larsxschneider]

From: Lars Schneider 

Add example implementation including test cases for the large file
system using Git LFS.

Pushing files to the Git LFS server is not tested.

Signed-off-by: Lars Schneider 
---
 Documentation/git-p4.txt |   4 +-
 git-p4.py|  52 ++
 t/t9823-git-p4-lfs.sh| 263 +++
 3 files changed, 318 insertions(+), 1 deletion(-)
 create mode 100755 t/t9823-git-p4-lfs.sh

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index e0d0239..3168a64 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -515,7 +515,9 @@ git-p4.largeFileSystem::
Please note that most of these large file systems depend on the
Git clean/smudge filters. These filters are not applied through
git-p4. You need to create a fresh clone of the repository after
-   running git-p4.
+   running git-p4. Only Git LFS [1] is supported right now. Download
+   and install the Git LFS command line extension to use this option.
+   [1] https://git-lfs.github.com/
 
 git-p4.largeFileExtensions::
All files matching a file extension in the list will be processed
diff --git a/git-p4.py b/git-p4.py
index 06651a8..d24d84f 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -935,6 +935,58 @@ def largeFileSystem():
 except AttributeError as e:
 die('Large file system not supported: %s' % 
gitConfig('git-p4.largeFileSystem'))
 
+class GitLFS:
+"""Git LFS as backend for the git-p4 large file system.
+   See https://git-lfs.github.com/ for details."""
+
+@staticmethod
+def attributeDescription():
+"""Return a description which is used to mark LFS entries in the
+   .gitattributes file."""
+return 'Git LFS (see https://git-lfs.github.com/)'
+
+@staticmethod
+def attributeFilter():
+"""Return the name of the filter which is used for LFS entries in the
+   .gitattributes file."""
+return 'lfs'
+
+@staticmethod
+def generatePointer(cloneDestination, contentFile):
+"""Generate a Git LFS pointer for the content. Return LFS Pointer file
+   mode and content which is stored in the Git repository instead of
+   the actual content. Return also the new location of the actual
+   content.
+   """
+pointerProcess = subprocess.Popen(
+['git', 'lfs', 'pointer', '--file=' + contentFile],
+stdout=subprocess.PIPE
+)
+pointerFile = pointerProcess.stdout.read()
+if pointerProcess.wait():
+os.remove(contentFile)
+die('git-lfs pointer command failed. Did you install the 
extension?')
+pointerContents = [i+'\n' for i in pointerFile.split('\n')[2:][:-1]]
+oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]
+oidPath = os.path.join(
+cloneDestination,
+'.git', 'lfs', 'objects', oid[:2], oid[2:4],
+oid,
+)
+# LFS Spec states that pointer files should not have the executable 
bit set.
+gitMode = '100644'
+return (gitMode, pointerContents, oidPath)
+
+@staticmethod
+def pushFile(contentFile):
+"""Push the actual content which is not stored in the Git repository to
+a server."""
+uploadProcess = subprocess.Popen(
+['git', 'lfs', 'push', '--object-id', 'origin', 
os.path.basename(contentFile)]
+)
+if uploadProcess.wait():
+die('git-lfs push command failed. Did you define a remote?')
+
 class Command:
 def __init__(self):
 self.usage = "usage: %prog [options]"
diff --git a/t/t9823-git-p4-lfs.sh b/t/t9823-git-p4-lfs.sh
new file mode 100755
index 000..4d58cac
--- /dev/null
+++ b/t/t9823-git-p4-lfs.sh
@@ -0,0 +1,263 @@
+#!/bin/sh
+
+test_description='Clone repositories and store files in Git LFS'
+
+. ./lib-git-p4.sh
+
+git lfs help >/dev/null 2>&1 || {
+   skip_all='skipping git p4 Git LFS tests; Git LFS not found'
+   test_done
+}
+
+test_file_in_lfs() {
+   FILE=$1
+   SIZE=$2
+   EXPECTED_CONTENT=$3
+   test_path_is_file $FILE &&
+   test_line_count = 3 $FILE &&
+   cat $FILE | grep "size $SIZE" &&
+   HASH=$(cat $FILE | grep "oid sha256:" | sed -e 's/oid sha256://g') &&
+   LFS_FILE=".git/lfs/objects/${HASH:0:2}/${HASH:2:2}/$HASH"
+   test_path_is_file $LFS_FILE &&
+   echo $EXPECTED_CONTENT >expect
+   test_cmp expect $LFS_FILE
+}
+
+test_expect_success 'start p4d' '
+   start_p4d
+'
+
+test_expect_success 'Create repo with binary files' '
+   client_view "//depot/... //client/..." &&
+   (
+   cd "$cli" &&
+
+   echo "txt 7b" >file1.txt &&
+   p4 add file1.txt &&
+   echo "bin 13 bytes" >file2.dat &&
+   p4 add file2.dat &&
+   p4 submit -d "Add text and binary file" 

[PATCH v3 1/5] git-p4: add optional type specifier to gitConfig reader

[larsxschneider]

From: Lars Schneider 

The functions “gitConfig” and “gitConfigBool” are almost identical. Make 
“gitConfig” more generic by adding an optional type specifier. Use the type 
specifier “—bool” with “gitConfig” to implement “gitConfigBool. This prepares 
the implementation of other type specifiers such as “—int”.

Signed-off-by: Lars Schneider 
---
 git-p4.py | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 073f87b..c139cab 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -604,9 +604,12 @@ def gitBranchExists(branch):
 
 _gitConfig = {}
 
-def gitConfig(key):
+def gitConfig(key, typeSpecifier=None):
 if not _gitConfig.has_key(key):
-cmd = [ "git", "config", key ]
+cmd = [ "git", "config" ]
+if typeSpecifier:
+cmd += [ typeSpecifier ]
+cmd += [ key ]
 s = read_pipe(cmd, ignore_error=True)
 _gitConfig[key] = s.strip()
 return _gitConfig[key]
@@ -617,10 +620,7 @@ def gitConfigBool(key):
in the config."""
 
 if not _gitConfig.has_key(key):
-cmd = [ "git", "config", "--bool", key ]
-s = read_pipe(cmd, ignore_error=True)
-v = s.strip()
-_gitConfig[key] = v == "true"
+_gitConfig[key] = gitConfig(key, '--bool') == "true"
 return _gitConfig[key]
 
 def gitConfigList(key):
-- 
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 3/5] git-p4: return an empty list if a list config has no values

[larsxschneider]

From: Lars Schneider 

Signed-off-by: Lars Schneider 
---
 git-p4.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/git-p4.py b/git-p4.py
index 40ad4ae..90d3b90 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -638,6 +638,8 @@ def gitConfigList(key):
 if not _gitConfig.has_key(key):
 s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
 _gitConfig[key] = s.strip().split(os.linesep)
+if _gitConfig[key] == ['']:
+_gitConfig[key] = []
 return _gitConfig[key]
 
 def p4BranchesInGit(branchesAreInRemotes=True):
-- 
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 2/5] git-p4: add gitConfigInt reader

[larsxschneider]

From: Lars Schneider 

Add a git config reader for integer variables. Please note that the git config 
implementation automatically supports k, m, and g suffixes.

Signed-off-by: Lars Schneider 
---
 git-p4.py | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/git-p4.py b/git-p4.py
index c139cab..40ad4ae 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -623,6 +623,17 @@ def gitConfigBool(key):
 _gitConfig[key] = gitConfig(key, '--bool') == "true"
 return _gitConfig[key]
 
+def gitConfigInt(key):
+if not _gitConfig.has_key(key):
+cmd = [ "git", "config", "--int", key ]
+s = read_pipe(cmd, ignore_error=True)
+v = s.strip()
+try:
+_gitConfig[key] = int(gitConfig(key, '--int'))
+except ValueError:
+_gitConfig[key] = None
+return _gitConfig[key]
+
 def gitConfigList(key):
 if not _gitConfig.has_key(key):
 s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
-- 
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Bug: git-upload-pack will return successfully even when it can't read all references

[Ævar Arnfjörð Bjarmason]

We have a process to back up our Git repositories at work, this
started alerting because it wasn't getting the same refs as the
remote.

This turned out to be a pretty trivial filesystem error.
refs/heads/master wasn't readable by the backup process, but some
other stuff in refs/heads and objects/* was.

But I think it's a bug that if we ssh to the remote end, and
git-upload-pack can't read certain refs in refs/heads/ that we don't
return an error.

This simple shellscript reproduces the issue:

rm -rf /tmp/repo /tmp/repo-checkout
git init /tmp/repo
cd /tmp/repo
touch foo
git add foo
git commit -m"foo"
git checkout -b branch
git checkout master
git show-ref
chmod 000 .git/refs/heads/master
git show-ref
cd /tmp
git clone repo repo-checkout
echo "Status code of clone: $?"
cd repo-checkout
git show-ref

After running this you get:

$ (cd /tmp/repo-checkout && echo -n | strace
/tmp/avar/bin/git-upload-pack /tmp/repo 2>&1 | grep -e EACCES)
open("refs/heads/master", O_RDONLY) = -1 EACCES (Permission denied)
open("refs/heads/master", O_RDONLY) = -1 EACCES (Permission denied)
open("refs/heads/master", O_RDONLY) = -1 EACCES (Permission denied)

And "git fetch" will return 0.

We fail to call get refs/heads/master in head_ref_namespaced() called
by upload_pack(). I was going to see if I could patch it to return an
error, but that code seems very far removed from any error checking.

This isn't only an issue with git-upload-pack, e.g. show-ref itself
has the same issue:

$ chmod 600 .git/refs/heads/master
$ git show-ref; echo $?
e7255c8fcabc6e15f57cd984f9f117870052c1a0 refs/heads/branch
e7255c8fcabc6e15f57cd984f9f117870052c1a0 refs/heads/master
0
$ chmod 000 .git/refs/heads/master
$ git show-ref; echo $?
e7255c8fcabc6e15f57cd984f9f117870052c1a0 refs/heads/branch
0

I wanted to check if this was a regression and got as far back as
v1.4.3 with the same behavior before the commands wouldn't work
anymore due to changes in the git config parsing code.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 0/2] contrib/subtree: make it respect spaces in a repository path

[Alexey Shumkin]

On Fri, Sep 04, 2015 at 04:08:06PM -0700, Junio C Hamano wrote:
> Alexey Shumkin  writes:
> 
> > Some repositories may have spaces in their paths. Currently `git-subtree`
> > raises an error in such cases.
> > Also, `git-subtree` currently does not have tests for its 'push' command.
> > Following patches are to fix these statements.
> >
> > Alexey Shumkin (2):
> >   t7900-subtree: test the "space in a subdirectory name" case
> >   contrib/subtree: respect spaces in a repository path
> 
> Doesn't this order break bisection?  It seems that you turn "subdir"
> to "sub dir" in existing tests, and I understand that the whole
> point of this series is that such a change will expose that the tool
> is broken, making tests fail.
It seems I have to reword commit messages to avoid such an interpretation.
Because, the first commit does not break anything. It is to change the
tests for `git-subtree` "to show" that `git-subtree`s already tested
commands (almost) work correctly if there are spaces in paths (except
--rejoin-msg issue).
And the second commit adds missing tests and the fix.
Should I add/commit the breaking test first and then commit the fix?
> 
> Also, if you feel up to it, it might be a good idea to clean t7900
> test up to the current best practice before doing any other changes
> as a pure preparatory clean-up patch.
> 
> Namely, using cd outside a subshell of the tests to move around is a
> bad thing to do, and you are adding more instance of it in this
> series.  If one test with such a cd to go down fails before it has a
> chance to come back up (or go up and then fail to come back down),
> the later tests will be left in an unexpected place.
I understand this issue with "cd" (I've just followed the existing t7900
tests "code style").
> 
> >  contrib/subtree/git-subtree.sh |   4 +-
> >  contrib/subtree/t/t7900-subtree.sh | 194 
> > +++--
> >  2 files changed, 124 insertions(+), 74 deletions(-)
> 
> Thanks.

-- 
Alexey Shumkin
E-mail: alex.crez...@gmail.com
ICQ: 118001447
Jabber (GoogleTalk): alex.crez...@gmail.com
Skype: crezoff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] filter-branch: add passed/remaining seconds on progress

[Gábor Bernát]

From: Gabor Bernat 

adds seconds progress and estimated seconds time if getting the current
timestamp is supported by the date %+s command

Signed-off-by: Gabor Bernat 
---

I've submitted this first to this list as a feature request, however
in the meantime with the help of Jeff King , Junio C
Hamano , Eric Sunshine 
and Mikael Magnusson  came up with solution, so now
I submit it as a revised patch.

The current solution updates the progress for all commits until 1
second time is elapsed. Afterwards updates it at most once a second.

Ammended build up as agreed at [1].

[1] http://thread.gmane.org/gmane.comp.version-control.git/277314
---
 git-filter-branch.sh | 32 +++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 5b3f63d..565144a 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -275,11 +275,41 @@ commits=$(wc -l <../revs | tr -d " ")
 test $commits -eq 0 && die "Found nothing to rewrite"
 
 # Rewrite the commits
+report_progress ()
+{
+if test -n "$progress"
+then
+   if test $git_filter_branch__commit_count -gt $next_sample_at
+   then
+   now_timestamp=$(date +%s)
+   elapsed_seconds=$(($now_timestamp - $start_timestamp))
+   remaining_second=$(( ($commits - 
$git_filter_branch__commit_count) * $elapsed_seconds / 
$git_filter_branch__commit_count ))
+   if test $elapsed_seconds -gt 0
+   then
+   next_sample_at=$(( ($elapsed_seconds + 1) * 
$git_filter_branch__commit_count / $elapsed_seconds ))
+   else
+   next_sample_at=$(($next_sample_at + 1))
+   fi
+   progress=" ($elapsed_seconds seconds passed, remaining 
$remaining_second predicted)"
+   fi
+fi
+printf "\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress"
+}
 
 git_filter_branch__commit_count=0
+
+progress= start_timestamp=
+if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'
+then
+   next_sample_at=0
+   progress="dummy to ensure this is not empty"
+   start_timestamp=$(date '+%s')
+fi
+
 while read commit parents; do
git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))
-   printf "\rRewrite $commit ($git_filter_branch__commit_count/$commits)"
+
+   report_progress
 
case "$filter_subdir" in
"")
-- 
2.6.0.rc0.3.gb3280a4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 0/2] git-p4: handle "Translation of file content failed"

[larsxschneider]

From: Lars Schneider 

Hi,

this patch fixes the P4 "Translation of file content failed" error. Unfortuantly
I was not able to generate a P4 test repository to reproduce the error with a
test case.

An Internet search shows that this error happens in the wild:
https://stackoverflow.com/questions/5156909/translation-of-file-content-failed-error-in-perforce
https://stackoverflow.com/questions/887006/perforce-translation-of-file-content-failed-error

Thanks,
Lars

Lars Schneider (2):
  git-p4: print stderr if P4 read_pipe operation fails
  git-p4: handle "Translation of file content failed"

 git-p4.py | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

--
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 2/2] git-p4: handle "Translation of file content failed"

[larsxschneider]

From: Lars Schneider 

A P4 repository can get into a state where it contains a file with file
type UTF16 that does not not contain valid UTF16 characters. If git-p4
attempts to retrieve the file as UTF16 from P4 then the process crashes
with a "Translation of file content failed" error.

Fix this by detecting this error and retrieving the file as binary
instead. The result in Git is the same.
---
 git-p4.py | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 36a4bcb..aaa0ad9 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2186,10 +2186,17 @@ class P4Sync(Command, P4UserMap):
 # them back too.  This is not needed to the cygwin windows version,
 # just the native "NT" type.
 #
-text = p4_read_pipe(['print', '-q', '-o', '-', "%s@%s" % 
(file['depotFile'], file['change']) ])
-if p4_version_string().find("/NT") >= 0:
-text = text.replace("\r\n", "\n")
-contents = [ text ]
+try:
+text = p4_read_pipe(['print', '-q', '-o', '-', '%s@%s' % 
(file['depotFile'], file['change'])])
+except Exception as e:
+if 'Translation of file content failed' in str(e):
+type_base = 'binary'
+else:
+raise e
+else:
+if p4_version_string().find('/NT') >= 0:
+text = text.replace('\r\n', '\n')
+contents = [ text ]
 
 if type_base == "apple":
 # Apple filetype files will be streamed as a concatenation of
-- 
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] filter-branch: add passed/remaining seconds on progress

[Gábor Bernát]

From: Gabor Bernat 

adds seconds progress and estimated seconds time if getting the current
timestamp is supported by the date +%s command

Signed-off-by: Gabor Bernat 
---

I've submitted this first to this list as a feature request, however
in the meantime with the help of Jeff King , Junio C
Hamano , Eric Sunshine 
and Mikael Magnusson  came up with solution, so now
I submit it as a revised patch.

The current solution updates the progress for all commits until 1
second time is elapsed. Afterwards updates it at most once a second.

Ammended build up as agreed at [1].

[1] http://thread.gmane.org/gmane.comp.version-control.git/277314
---
 git-filter-branch.sh | 32 +++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 5b3f63d..565144a 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -275,11 +275,41 @@ commits=$(wc -l <../revs | tr -d " ")
 test $commits -eq 0 && die "Found nothing to rewrite"
 
 # Rewrite the commits
+report_progress ()
+{
+if test -n "$progress"
+then
+   if test $git_filter_branch__commit_count -gt $next_sample_at
+   then
+   now_timestamp=$(date +%s)
+   elapsed_seconds=$(($now_timestamp - $start_timestamp))
+   remaining_second=$(( ($commits - 
$git_filter_branch__commit_count) * $elapsed_seconds / 
$git_filter_branch__commit_count ))
+   if test $elapsed_seconds -gt 0
+   then
+   next_sample_at=$(( ($elapsed_seconds + 1) * 
$git_filter_branch__commit_count / $elapsed_seconds ))
+   else
+   next_sample_at=$(($next_sample_at + 1))
+   fi
+   progress=" ($elapsed_seconds seconds passed, remaining 
$remaining_second predicted)"
+   fi
+fi
+printf "\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress"
+}
 
 git_filter_branch__commit_count=0
+
+progress= start_timestamp=
+if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'
+then
+   next_sample_at=0
+   progress="dummy to ensure this is not empty"
+   start_timestamp=$(date '+%s')
+fi
+
 while read commit parents; do
git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))
-   printf "\rRewrite $commit ($git_filter_branch__commit_count/$commits)"
+
+   report_progress
 
case "$filter_subdir" in
"")
-- 
2.6.0.rc0.3.gb3280a4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: bash completion lacks options

[Ævar Arnfjörð Bjarmason]

On Mon, Sep 7, 2015 at 5:07 PM, Olaf Hering  wrote:
> "git send-email --f" lacks --find-renames and others. Is the list
> of possible options maintained manually?

Yes, see contrib/completion/git-completion.bash.

There's no code for send-email there, you (or someone) could submit a patch! :)

> Perhaps this should be
> automated by placing the long strings in an ELF section, then filling
> variables like $__git_format_patch_options from such ELF section.
> An example how this was done in libguestfs is here (see daemon/daemon.h):
> https://github.com/libguestfs/libguestfs/commit/0306c98d319d189281af3c15101c8d343e400f13

This is an interesting approach, but wouldn't help with git-send-email
in particular, it's a Perl script, so there's no ELF section to parse.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: bash completion lacks options

[Olaf Hering]

Am 07.09.2015 um 17:34 schrieb Ævar Arnfjörð Bjarmason:
> On Mon, Sep 7, 2015 at 5:07 PM, Olaf Hering  wrote:

>> https://github.com/libguestfs/libguestfs/commit/0306c98d319d189281af3c15101c8d343e400f13
> 
> This is an interesting approach, but wouldn't help with git-send-email
> in particular, it's a Perl script, so there's no ELF section to parse.

format-patch is a ELF binary, a link to git itself as I notice
just now.

Olaf
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/5] enter_repo: avoid duplicating logic, use is_git_directory() instead

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---

The cover letter talks about "local clone", and in this entire
series, I saw new tests only for the local case, but doesn't this
and the next change also affect the case where a Git daemon or a
upload-pack process is serving the remote repository?

And if so, how is that case affected?

>  path.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/path.c b/path.c
> index a536ee3..7340e11 100644
> --- a/path.c
> +++ b/path.c
> @@ -441,8 +441,7 @@ const char *enter_repo(const char *path, int strict)
>   else if (chdir(path))
>   return NULL;
>  
> - if (access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
> - validate_headref("HEAD") == 0) {
> + if (is_git_directory(".")) {
>   set_git_dir(".");
>   check_repository_format();
>   return path;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/5] clone: allow --local from a linked checkout

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> Noticed-by: Bjørnar Snoksrud 
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  builtin/clone.c | 6 --
>  t/t2025-worktree-add.sh | 5 +
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 578da85..836fb64 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -424,8 +424,10 @@ static void clone_local(const char *src_repo, const char 
> *dest_repo)
>   } else {
>   struct strbuf src = STRBUF_INIT;
>   struct strbuf dest = STRBUF_INIT;
> - strbuf_addf(, "%s/objects", src_repo);
> - strbuf_addf(, "%s/objects", dest_repo);
> + get_common_dir(, src_repo);
> + get_common_dir(, dest_repo);
> + strbuf_addstr(, "/objects");
> + strbuf_addf(, "/objects");

Why addstr vs addf?  Shouldn't both be addstr?

>   copy_or_link_directory(, , src_repo, src.len);
>   strbuf_release();
>   strbuf_release();
> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> index 8267411..3694174 100755
> --- a/t/t2025-worktree-add.sh
> +++ b/t/t2025-worktree-add.sh
> @@ -193,4 +193,9 @@ test_expect_success '"add" -B/--detach mutually 
> exclusive' '
>   test_must_fail git worktree add -B poodle --detach bamboo master
>  '
>  
> +test_expect_success 'local clone from linked checkout' '
> + git clone --local here here-clone &&
> + ( cd here-clone && git fsck )
> +'
> +
>  test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

bash completion lacks options

[Olaf Hering]

"git send-email --f" lacks --find-renames and others. Is the list
of possible options maintained manually? Perhaps this should be
automated by placing the long strings in an ELF section, then filling
variables like $__git_format_patch_options from such ELF section.
An example how this was done in libguestfs is here (see daemon/daemon.h):
https://github.com/libguestfs/libguestfs/commit/0306c98d319d189281af3c15101c8d343e400f13

Olaf
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs

[Matthieu Moy]

Karthik Nayak  writes:

> On Mon, Sep 7, 2015 at 12:03 PM, Junio C Hamano  wrote:
>>> diff --git a/builtin/tag.c b/builtin/tag.c
>>> index 9fa1400..f55dfda 100644
>>> --- a/builtin/tag.c
>>> +++ b/builtin/tag.c
>>> @@ -43,8 +43,8 @@ static int list_tags(struct ref_filter *filter, struct 
>>> ref_sorting *sorting, con
>>>
>>>   if (!format) {
>>>   if (filter->lines)
>>> - format = to_free = 
>>> xstrfmt("%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)",
>>> -filter->lines);
>>> + format = to_free = 
>>> xstrfmt("%%(align:15,left)%%(refname:short)%%(end) "
>>> +"%%(contents:lines=%d)", 
>>> filter->lines);
>>
>> This line still looks overlong.  Would it help to stop spelling this
>> as a double "a = b = overlong expression" assignment?
>>
>
> I'm not sure, I get what you mean.

I guess

format = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) 
%%(contents:lines=%d)",
 filter->lines);
to_free = format;

(still 83 columns + indentation, but that's a bit shorter than your
version).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] filter-branch: add passed/remaining seconds on progress

[Ramsay Jones]

On 07/09/15 13:31, Gábor Bernát wrote:
> From: Gabor Bernat 
>
> adds seconds progress and estimated seconds time if getting the current
> timestamp is supported by the date %+s command

s/%+s/+%s/

ATB,
Ramsay Jones


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs

[Karthik Nayak]

On Mon, Sep 7, 2015 at 12:03 PM, Junio C Hamano  wrote:
> Karthik Nayak  writes:
>
>> diff --git a/Documentation/git-for-each-ref.txt 
>> b/Documentation/git-for-each-ref.txt
>> index d039f40..c5154bb 100644
>> --- a/Documentation/git-for-each-ref.txt
>> +++ b/Documentation/git-for-each-ref.txt
>> @@ -128,13 +128,14 @@ color::
>>   are described in `color.branch.*`.
>>
>>  align::
>> - Left-, middle-, or right-align the content between %(align:..)
>> + Left-, middle-, or right-align the content between %(align:...)
>>   and %(end). Followed by `:,`, where the
>>   `` is either left, right or middle and `` is
>>   the total length of the content with alignment. If the
>>   contents length is more than the width then no alignment is
>>   performed. If used with '--quote' everything in between
>> - %(align:..)  and %(end) is quoted.
>> + %(align:...) and %(end) is quoted, but if nested then only the
>> + topmost level performs quoting.
>
> I am not sure if the description of quote belongs to "align".  Isn't
> the general rule at the endgame when there are more opening things
> that would buffer its effect up to the corresponding %(end):
>
>  - Some atoms like %(align) and %(if) always require a matching
>%(end).  We call them "opening atoms" and sometimes denote them
>as %($open).
>
>  - When a scripting language specific quoting is in effect, except
>for opening atoms, replacement from every %(atom) is quoted when
>and only when it appears at the top-level (that is, when it
>appears outside %($open)...%(end)).
>
>  - When a scripting language specific quoting is in effect,
>everything between a top-level opening atom and its matching
>%(end) is evaluated according to the semantics of the opening
>atom and its result is quoted.
>
> To put the third item above in a different way, a matching
> %($open)...  %(end) pair behaves as if it is a single normal atom,
> from the point of view of the quoting rule.  All top-level atoms are
> invidivually quoted, whether they are normal atoms or open/end pairs.
> And that rule is not limited to %(align).
>
> I am flexible with the terminology, but the point is that I think
> the quoting rules are better be specified _outside_ the description
> of a particular atom, but as a general rule.
>

I definitely agree, but like Matthieu said, corrently we have only
one such atom and it makes sense to note this behaviour under that.
When we get %(if) to work we could move this over to a more general
section?

>> diff --git a/builtin/tag.c b/builtin/tag.c
>> index 9fa1400..f55dfda 100644
>> --- a/builtin/tag.c
>> +++ b/builtin/tag.c
>> @@ -43,8 +43,8 @@ static int list_tags(struct ref_filter *filter, struct 
>> ref_sorting *sorting, con
>>
>>   if (!format) {
>>   if (filter->lines)
>> - format = to_free = 
>> xstrfmt("%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)",
>> -filter->lines);
>> + format = to_free = 
>> xstrfmt("%%(align:15,left)%%(refname:short)%%(end) "
>> +"%%(contents:lines=%d)", 
>> filter->lines);
>
> This line still looks overlong.  Would it help to stop spelling this
> as a double "a = b = overlong expression" assignment?
>

I'm not sure, I get what you mean.

>>   /*
>>* Perform quote formatting when the stack element is that of
>> -  * a modifier atom and right above the first stack element.
>> +  * a supporting atom. If nested then perform quote formatting
>> +  * only on the topmost supporting atom.
>>*/
>>   if (!state->stack->prev->prev) {
>>   quote_formatting(, current->output.buf, state->quote_style);
>> @@ -261,6 +262,22 @@ static void end_atom_handler(struct atom_value *atomv, 
>> struct ref_formatting_sta
>>   pop_stack_element(>stack);
>>  }
>>
>> +static int match_atom_name(const char *name, const char *atom_name, const 
>> char **val)
>> +{
>> + const char *body;
>> +
>> + if (!skip_prefix(name, atom_name, ))
>> + return 0; /* doesn't even begin with "atom_name" */
>> + if (!body[0] || !body[1]) {
>> + *val = NULL; /* %(atom_name) and no customization */
>> + return 1;
>> + }
>
> This if condition is puzzling.  !body[0] would mean the name was
> exactly atom_name, which is what you want to catch.
>
> But the other part does not make any sense to me.  what does
> (body[0] != '\0' && !body[1]) mean?  atom_name asked for was "align"
> and name was "aligna"?  That is not "%(align) and no customization".
>

The idea was to ensure that the atom doesn't end at the ':'.

>> + if (body[0] != ':')
>> + return 0; /* "atom_namefoo" is not "atom_name" or 
>> "atom_name:..." */
>
> This one makes sense to me.
>
>> + *val = body + 1; /* "atomname:val" */
>
> 

Improving auto conflict resolving while merge

[KES]

Hi.

**To not apply big patches, I will supply links to public repository

Lets start

1. git clone https://github.com/KES777/Plack

2. git show ed485bf75a6

Nothing interesting. Usual merge conflict while merge branch 
'feature/doc_contribute' to 'master'

But I was interested by this part:

   my $app = sub {
   my $env =  shift;
 
+<<< HEAD
+===
+  my $app = sub {
+  my $env =  shift;
+
+>>> master

I know that some code were cherry picked(??) by maintainer from by branch. Ans 
this is the part which are same in master and branch.
So I not expect conflict here. I start to dig.

I compare Log4perl.pm files from branch and master 
3. git checkout feature/doc_contribution
4. cd lib/Plack/Middleware
5. git diff d095870 Log4perl.pm

I see that both master and branch has same resulting changes:
@@ -36,10 +35,10 @@ __END__
 
 =head1 NAME
 
-Plack::Middleware::Log4perl - Uses Log::Log4perl to configure logger
+Plack::Middleware::Log4perl - Uses Log::Log4perl to configure psgix.logger
 
 =head1 SYNOPSIS
-
+  # How to use logger in your app
   my $app = sub {
   my $env =  shift;
 
@@ -53,18 +52,25 @@ Plack::Middleware::Log4perl - Uses Log::Log4perl to 
configure logger

But this part of code were edited in both branches too:

The branch were started at this commit: 0a5ff8427

6. git diff 0a5ff8427

@@ -35,14 +35,28 @@ __END__
 
 =head1 NAME
 
-Plack::Middleware::Log4perl - Uses Log::Log4perl to configure logger
+Plack::Middleware::Log4perl - Uses Log::Log4perl to configure psgix.logger
 
 =head1 SYNOPSIS
+  # How to use logger in your app
+  my $app = sub {
+  my $env =  shift;
 
-  use Log::Log4perl;
+  $env->{'psgix.logger'}({ level => 'error', message => 'Hi' });
+
+  return [
+  '200',
+  [ 'Content-Type' => 'text/plain' ],
+  [ "Hello World" ],
+  ];
+  };

7. git checkout master 
8. git diff 0a5ff8427

@@ -39,28 +40,41 @@ Plack::Middleware::Log4perl - Uses Log::Log4perl to 
configure logger
 
 =head1 SYNOPSIS
 
-  use Log::Log4perl;
+  my $app = sub {
+  my $env =  shift;
+
+  $env->{'psgix.logger'}({ level => 'error', message => 'Hi' });
+
+  return [
+  '200',
+  [ 'Content-Type' => 'text/plain' ],
+  [ "Hello World" ],
+  ];
+  };
+

I repeat for emphasis. In the resulting file we have same code and not expect 
conflict.

At this point I have same changes on master and branch.
master head is d095870
branch head is a3db36a
fork point is 0a5ff84

Comparing step-by-step 
(A)   git diff 0a5ff84..d095870
(B)   git diff 0a5ff84..a3db36a
Find patches which apply to same lines
 =head1 SYNOPSIS
 
-  use Log::Log4perl;
+  my $app = sub {
+  my $env =  shift;
+
+  $env->{'psgix.logger'}({ level => 'error', message => 'Hi' });
+
+  return [
+  '200',
+  [ 'Content-Type' => 'text/plain' ],
+  [ "Hello World" ],
+  ];
+  };
+

 =head1 SYNOPSIS
+  # How to use logger in your app
+  my $app = sub {
+  my $env =  shift;
 
-  use Log::Log4perl;
+  $env->{'psgix.logger'}({ level => 'error', message => 'Hi' });
+
+  return [
+  '200',
+  [ 'Content-Type' => 'text/plain' ],
+  [ "Hello World" ],
+  ];
+  };

And check with 
(C)  git diff a3db36a..d095870
 =head1 SYNOPSIS
-  # How to use logger in your app
+
   my $app = sub {
   my $env =  shift;
 
(D) So this merge conflict (the result of `git checkout master; git merge 
feature/doc_contribution`)
@@@ -36,13 -35,13 +36,19 @@@ __END_
  
  =head1 NAME
  
- Plack::Middleware::Log4perl - Uses Log::Log4perl to configure logger
+ Plack::Middleware::Log4perl - Uses Log::Log4perl to configure psgix.logger
  
  =head1 SYNOPSIS
+   # How to use logger in your app
+   my $app = sub {
+   my $env =  shift;
  
++<<< HEAD
 +  my $app = sub {
 +  my $env =  shift;
 +
++===
++>>> feature/doc_contribution
$env->{'psgix.logger'}({ level => 'error', message => 'Hi' });
  
return [

can be simplified to:

@@@ -36,13 -35,13 +36,19 @@@ __END_
  
  =head1 NAME
  
- Plack::Middleware::Log4perl - Uses Log::Log4perl to configure logger
+ Plack::Middleware::Log4perl - Uses Log::Log4perl to configure psgix.logger
  
  =head1 SYNOPSIS
+   # How to use logger in your app
my $app = sub {
my $env =  shift;
  

NEXT
Next merge conflict seems this:
(A)
+  };
+
 
+  # Use your own Log4perl configuration
+  use Log::Log4perl;
   Log::Log4perl::init('/path/to/log4perl.conf');

(B)
+  };
 
+
+  # Initialization. Case#1
+  use Log::Log4perl;
   Log::Log4perl::init('/path/to/log4perl.conf');

Comparing to (C)
@@ -53,18 +52,25 @@
   };
 
 
-  # Use your own Log4perl configuration
+  # Initialization. Case#1
   use Log::Log4perl;
   Log::Log4perl::init('/path/to/log4perl.conf');

So current merge conflict (D) 
@@@ -51,20 -50,27 +57,33 @@@
[ "Hello World" ],
];
};
++<<< HEAD
 +
 +
 +  # Use your own Log4perl configuration
++===
+ 
+ 
+   # 

Conditionally define vars to improve portability

[Renato Botelho]

Default variables used to build are set using = on Makefile, (e.g. CC, INSTALL, 
CFLAGS, …). GNU make overwrite these values if it’s passed as an argument (make 
CC=clang) and it works as expected.

Default method of passing arguments for make operations on FreeBSD ports tree 
is using environment variables instead of make arguments, then we have CC set 
on env before call gmake. Today these values are ignored by git Makefile, and 
we ended up patching Makefile replacing = by ?= on variable assignments [1].

Before I write a patch and submit I would like to check if it would be an 
acceptable change of if it’s something you won’t accept for any reason.

Regards

[1] 
https://svnweb.freebsd.org/ports/head/devel/git/files/patch-Makefile?revision=396048=markup#l7
--
Renato Botelho

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v16 05/14] ref-filter: introduce match_atom_name()

[Karthik Nayak]

On Mon, Sep 7, 2015 at 1:22 AM, Eric Sunshine  wrote:
> On Sat, Sep 5, 2015 at 2:52 PM, Karthik Nayak  wrote:
>> Introduce match_atom_name() which helps in checking if a particular
>> atom is the atom we're looking for and if it has a value attached to
>> it or not.
>>
>> Use it instead of starts_with() for checking the value of %(color:...)
>> atom. Write a test for the same.
>>
>> Signed-off-by: Karthik Nayak 
>> ---
>> diff --git a/ref-filter.c b/ref-filter.c
>> index a993216..e99c342 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> +static int match_atom_name(const char *name, const char *atom_name, const 
>> char **val)
>> +{
>> +   const char *body;
>> +
>> +   if (!skip_prefix(name, atom_name, ))
>> +   return 0; /* doesn't even begin with "atom_name" */
>> +   if (!body[0] || !body[1]) {
>> +   *val = NULL; /* %(atom_name) and no customization */
>> +   return 1;
>
> If this is invoked as match_atom_name("colors", "color", ...), then it
> will return true (matched, but no value), which is not correct at all;
> "colors" is not a match for atom %(color). Maybe you meant?
>
> if (!body[0] || (body[0] == ':' && !body[1])) {
>
> But, that's getting ugly and complicated, and would be bettered
> handled by reordering the logic of this function for dealing with the
> various valid and invalid cases. However...
>

Well as you infered the logic is to check so that there is something
existing after the ':'. About why I didn't do something like this
"(body[0] == ':' && !body[1])" is because ref-filter already
checks for invalid atom names.

So before even match_atom_name() is called, the check is done in
parse_ref_filter_atom().

>> +   }
>> +   if (body[0] != ':')
>> +   return 0; /* "atom_namefoo" is not "atom_name" or 
>> "atom_name:..." */
>> +   *val = body + 1; /* "atomname:val" */
>> +   return 1;
>> +}
>
> It's not clear why this function exists in the first place. It's only
> purpose seems to be to specially recognize instances of atoms which
> should have a ":" but lack it, so that the caller can then report an
> error.
>
> But why is this better than the more generic solution of merely
> reporting an error for *any* unrecognized atom? How does it help to
> single out these special cases?
>
> There is a further inconsistency in that you are only specially
> recognizing %(color) and %(align) lacking ":", but not
> %(content:lines) lacking "=" in patch 8/14. Why do %(color:...) and
> %(align:...) get special treatment but %(content:lines=...) does not?
> Such inconsistency again argues in favor of a generic "unrecognized
> atom" detection and reporting over special case handling.
>

This is from http://article.gmane.org/gmane.comp.version-control.git/277099
Junio suggested to have this to check for ":" rather than rely on
skip_prefix() and check manually after that.

Agreed, a more generic solution would be better, and If I remember I said,
I'll work on that after this entire series.

About contents:lines, we declare contents:lines itself as an atom, this was to
keep it similar to how the other contents atoms are declared, so the testing
for this again is already done by parse_ref_filter_atom().

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Subdividing an existing Repository

[David A Cobb]

Several Repositories I'm interested in, e.g. MINGW-PACKAGES@GITHUB, 
contain multiple "products."  Usually, I'm only interested in one or two.
I want to have a local repo with, e.g. iodbc from MinGW-Packages as 
sub-moules.  I don't even need to keep local content for many of the 
other included products.  But, I need to keep synchronized with the 
upstream, so I don't want to destroy the meta-data that allows me to do 
that.


It's not certain at this point that I will ever be invited to push what 
I have done up the food chain.  Maybe it will always be a private fork.


I've looked at Subtrees and Submodules in the ProGit book.  I can see 
how to use an existing submodule, but not how to isolate one out of a 
larger body of code.


TIA
<>

Re: Questions about git-push for huge repositories

[Jeff King]

On Mon, Sep 07, 2015 at 09:05:41AM +0800, Levin Du wrote:

> > Instead, the object transfer is optimized by comparing what commits
> > each side has and sending trees and blobs that are reachable from
> > the commits that the receiving side does not have.
> 
> The sender A sends all the commits that the receiver B does not have.
> The commits contains trees and blobs. In my situation, branch in A has
> only one commit. It seems that B has received lots of duplicate blobs,
> concluded from the GC result.

Right. B tells A "I already have this commit", but A does not already
have it, so that information is not helpful. It cannot make any
assumptions about what B has, and must send all trees and blobs
referenced by its commit.

> What I do not understand is, how duplicate blobs happen in a git repository?
> Git repository is famous for its content addressing storage system.
> I guess that A sends its packed file to B directly, no matter what are
> already in B.

Not exactly.  During a push, git may or may not keep the packfile sent
over the wire, depending on the number of objects in it and the
receive.unpackLimit config setting. The same object can exist in two
separate packfiles. One of the effects of "git gc" is to remove such
duplicates.

So A effectively does send its whole pack in this case, but only because
it cannot find any shared history with B (and B keeps it as-is until the
next gc because it is over the unpackLimit).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH, RESEND] gitk: adjust the menu line numbers to compensate for the new entry

[Beat Bolli]

Commit d835dbb9 ("gitk: Add a "Copy commit summary" command",
2015-08-13) in the upstream gitk repo added a new context menu entry.
Therefore, the line numbers of the entries below the new one need to be
adjusted when their text or state is changed.

Signed-off-by: Beat Bolli 
Cc: Paul Mackerras 
---
 gitk-git/gitk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index d05169a..bc0e586 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -8877,13 +8877,13 @@ proc rowmenu {x y id} {
 if {$id ne $nullid && $id ne $nullid2} {
set menu $rowctxmenu
if {$mainhead ne {}} {
-   $menu entryconfigure 7 -label [mc "Reset %s branch to here" 
$mainhead] -state normal
+   $menu entryconfigure 8 -label [mc "Reset %s branch to here" 
$mainhead] -state normal
} else {
-   $menu entryconfigure 7 -label [mc "Detached head: can't reset" 
$mainhead] -state disabled
+   $menu entryconfigure 8 -label [mc "Detached head: can't reset" 
$mainhead] -state disabled
}
-   $menu entryconfigure 9 -state $mstate
$menu entryconfigure 10 -state $mstate
$menu entryconfigure 11 -state $mstate
+   $menu entryconfigure 12 -state $mstate
 } else {
set menu $fakerowmenu
 }
-- 
2.5.0.492.g918e48c
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Bug in default commit hook (improperly forbidding a single blank line at EOF)

[Jeff King]

On Mon, Sep 07, 2015 at 06:37:29PM -0700, Raymond Jennings wrote:

> Please see https://bugs.gentoo.org/show_bug.cgi?id=559920 for further
> details.
> 
> Files *should* have a single blank line at the end, because a line should
> always have a newline at the end.

I'm not sure I follow. Lines should have a newline at the end, but there
is no need to start a _new_ blank line. So a file with zero bytes has no
lines (and no newline).

A file that contains a single line, like "one\n", has each line end in a
newline, and the file ends in a newline. There is no blank line.

A file like "one\n\n" has two lines: one with text, and a blank line at
the end.

Can you clarify (preferably by showing a byte sequence of the file in
question) what file you are feeding to the hook, what output you get,
and what output you expect?

> Adding a newline to the end of a file whose last line doesn't have one
> should be legal...as long as you don't create empty lines at the end.

If you mean turning "foo" (a file with no newline!) into "foo\n", I
agree that is legal, and does not create an empty blank line at the end.
But I don't think the hook complains about that.

E.g., we can create a sequence of file content:

  git init

  echo -n one >file
  git add file
  git commit -m 'no newline'

  echo >>file
  git add file
  git commit -m 'complete line'

  echo >>file
  git add file
  git commit -m 'add a blank line'

and run "log --check", which runs the same code that the pre-commit hook
does:

  git log --check

Git complains only about the final, which looks right to me. If you want
to redefine git's idea of which whitespace is worth complaining about,
try:

  git config core.whitespace -blank-at-eof

See the description of core.whitespace in "git help config" for the
complete list.  You can also set it per-file using gitattributes. See
"git help attributes", section "Checking whitespace errors".

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Bug in default commit hook (improperly forbidding a single blank line at EOF)

[Raymond Jennings]

Please see https://bugs.gentoo.org/show_bug.cgi?id=559920 for further 
details.


Files *should* have a single blank line at the end, because a line 
should always have a newline at the end.


Adding a newline to the end of a file whose last line doesn't have one 
should be legal...as long as you don't create empty lines at the end.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Bug in default commit hook (improperly forbidding a single blank line at EOF)

[Raymond Jennings]

On 09/07/15 21:55, Jeff King wrote:

On Mon, Sep 07, 2015 at 06:37:29PM -0700, Raymond Jennings wrote:


Please see https://bugs.gentoo.org/show_bug.cgi?id=559920 for further
details.

Files *should* have a single blank line at the end, because a line should
always have a newline at the end.

I'm not sure I follow. Lines should have a newline at the end, but there
is no need to start a _new_ blank line. So a file with zero bytes has no
lines (and no newline).

A file that contains a single line, like "one\n", has each line end in a
newline, and the file ends in a newline. There is no blank line.

A file like "one\n\n" has two lines: one with text, and a blank line at
the end.

Can you clarify (preferably by showing a byte sequence of the file in
question) what file you are feeding to the hook, what output you get,
and what output you expect?


Adding a newline to the end of a file whose last line doesn't have one
should be legal...as long as you don't create empty lines at the end.

If you mean turning "foo" (a file with no newline!) into "foo\n", I
agree that is legal, and does not create an empty blank line at the end.
But I don't think the hook complains about that.
Sorry, my mistake.  I just took a look at the file on console with 
mcedit, and it looks like gedit lied to me.


I'll be contacting gedit's maintainers with this instead, sorry for the 
spam.

E.g., we can create a sequence of file content:

   git init

   echo -n one >file
   git add file
   git commit -m 'no newline'

   echo >>file
   git add file
   git commit -m 'complete line'

   echo >>file
   git add file
   git commit -m 'add a blank line'

and run "log --check", which runs the same code that the pre-commit hook
does:

   git log --check

Git complains only about the final, which looks right to me. If you want
to redefine git's idea of which whitespace is worth complaining about,
try:

   git config core.whitespace -blank-at-eof

See the description of core.whitespace in "git help config" for the
complete list.  You can also set it per-file using gitattributes. See
"git help attributes", section "Checking whitespace errors".

-Peff


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Questions about git-push for huge repositories

[Jeff King]

On Tue, Sep 08, 2015 at 09:30:09AM +0800, Levin Du wrote:

> Take kernel source code for example:
> 
> # Clone the kernel to A and B
> $ git --version
> git version 2.3.2
> $ git clone --bare  ../kernel/ A
> $ git clone --bare  ../kernel/ B

OK, two repos with the same source.

> # Create the orphan commit and check
> $ cd A
> $ git branch test
> Switched to a new branch 'test'
> $ git replace --graft test
> $ git rev-parse test
> cbbae6741c60c9e09f87521e3a79810abd6a2fda
> $ git rev-parse test^{tree}
> 929bdce0b48ca6079ad281a9d8ba24de3e49881a
> $ git rev-parse replace/cbbae6741c60c9e09f87521e3a79810abd6a2fda
> 82d3e9ce1ca062c219f1209c5291ccd5603e5302
> $ git rev-parse 82d3e9ce1ca062c219f1209c5291ccd5603e5302^{tree}
> 929bdce0b48ca6079ad281a9d8ba24de3e49881a
> $ git log --pretty=oneline 82d3e9ce1ca062c219f1209c5291ccd5603e5302 | wc -l
> 1

So you've created a new commit object, 82d3e9ce1, which has the same
tree as the original branch, but no parents.

Note that fetch and push do not respect the "replace" mechanism. They
can't, because we have no idea if the other side of the connection
shares our "replace" view of the world. So if I use "replace" to say
that commit X has parent Y, I cannot assume that pushing to some _other_
repository with X means that they also have all of Y.

But it should be OK, of course, to push the new orphan commit. I.e., if
we are pushing the object itself, not caring that it is part of a
"replace" mechanism, that should be no different than pushing any other
commit.

> $ du -hs ../B
> 1.6G ../B
> $ git push ../B 'refs/replace/*'
> Counting objects: 51216, done.
> Delta compression using up to 8 threads.
> Compressing objects: 100% (48963/48963), done.
> Writing objects: 100% (51216/51216), 139.61 MiB | 17.88 MiB/s, done.
> Total 51216 (delta 3647), reused 34580 (delta 1641)
> To ../B
> * [new branch]
> refs/replace/cbbae6741c60c9e09f87521e3a79810abd6a2fda ->
> refs/replace/cbbae6741c60c9e09f87521e3a79810abd6a2fda
> $ du -hs ../B
> 1.7G ../B
> 
> It takes some time for 'git push' to compress the objects and B has
> finally increased 0.1G,
> which is for the newly commit whose tree is already in the repository.

Right, this is due to the commit-walking that Junio explained earlier.
We walk the commits only, and then expand the positive side (things the
other side wants) into trees and blobs. Even though we know about a
commit that the other side has that points to the tree, we don't make
the connection.

You can get a more thorough answer by expanding and marking all trees
and blobs, taking the set difference between all of the objects you want
to send, and all of the objects you know the other side has. I.e.,
basically:

  # what we want to send
  git rev-list --objects 82d3e9ce1ca062c219f1209c5291ccd5603e5302 | sort >want

  # what we know the other side has; turn off replacements, since we
  # want the real value, not with our fake replace overlaid
  git --no-replace-objects rev-list --objects refs/heads/master | sort >have

  # set difference
  comm -23 want have

which should consist of only the one commit. But if you actually ran
that, you may notice that the second rev-list takes a long time to run.
In your exact case, one can get lucky by progressively drilling down
into commits and their trees (since the tip commit of "master" happens
to share the identical tree with our new fake commit). But that is
rather an uncommon example, and in more normal cases of fetching from
somebody, building on top, and then pushing back up, it is much more
expensive. In those cases it is much more efficient to walk the small
number of new commits and then expand only their newly-added objects.

If you turn on reachability bitmaps, git _will_ do the thorough set
difference, because it becomes much cheaper to do so. E.g., try:

git repack -adb

in repo A to build a single pack with bitmaps enabled. Then a subsequent
push should send only a single object (the new commit).

Of course the time spent building the bitmaps is larger than a single
push, so this is not a good strategy if you are just trying to send one
tree.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs

[Karthik Nayak]

On Mon, Sep 7, 2015 at 7:35 PM, Matthieu Moy
 wrote:
> Karthik Nayak  writes:
>
>> On Mon, Sep 7, 2015 at 12:03 PM, Junio C Hamano  wrote:
 diff --git a/builtin/tag.c b/builtin/tag.c
 index 9fa1400..f55dfda 100644
 --- a/builtin/tag.c
 +++ b/builtin/tag.c
 @@ -43,8 +43,8 @@ static int list_tags(struct ref_filter *filter, struct 
 ref_sorting *sorting, con

   if (!format) {
   if (filter->lines)
 - format = to_free = 
 xstrfmt("%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)",
 -filter->lines);
 + format = to_free = 
 xstrfmt("%%(align:15,left)%%(refname:short)%%(end) "
 +"%%(contents:lines=%d)", 
 filter->lines);
>>>
>>> This line still looks overlong.  Would it help to stop spelling this
>>> as a double "a = b = overlong expression" assignment?
>>>
>>
>> I'm not sure, I get what you mean.
>
> I guess
>
> format = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) 
> %%(contents:lines=%d)",
>  filter->lines);
> to_free = format;
>
> (still 83 columns + indentation, but that's a bit shorter than your
> version).

Also we could drop left, its default anyways.

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC] Pinning of submodules

[Anders Ro]

On 07/09/15 22:13, Jens Lehmann wrote:
> Am 07.09.2015 um 01:43 schrieb Eric Sunshine:
>> On Sun, Sep 6, 2015 at 6:08 PM, Anders Ro 
>> wrote:
>>> On 04/09/15 07:02, Eric Sunshine wrote:
 On Wed, Sep 2, 2015 at 7:34 PM, Anders Ro
  wrote:
> git-submodule.sh: pin submodule when branch name is '@'
>
> Setting branch name to '@' for a submodule will disable 'git submodule
> update --remote' calls for that specific submodule. I.e. instead of
> follow the unspecified default choice of master, nothing is being
> updated. This is useful when multiple submodules exist but not all
> should follow the remote branch head.

 With the disclaimer that I'm not a submodule user (to which the
 answer might be obvious): What benefit is there in using a magic
 value like this ("@") over, say, an explicit configuration setting?
>>>
>>>  From what I have understood (not a submodule expert yet) the '@' is an
>>> invalid branch name and should therefore not collide with any current
>>> branches. My idea was to disable the '--remote' option when the user
>>> have explicitly set an invalid branch name to not modify any current
>>> behaviour. Though having an explicit option is of course more
>>> clarifying. The current behaviour though is that empty branch name means
>>> "follow master" which is somewhat unintuitive.
>>
>> My concern in asking was that some future person might come up with
>> another scenario which also wants to use a "magic value" and would
>> have to invent / arrive at another "illegal" representation. Hence, an
>> explicit setting might be more appropriate. However, as stated, I
>> don't even use submodules, so I may be far off the mark. I've cc'd a
>> few of the submodule maintainers with the hope that they will chime
>> in.

Agree this is not a "future proof" solution. Though faily a quick one. I
started with a setting but realized it involved a bit more changes since
you should be able to tell the submodule command to pin a submodule
right from the start etc.

> 
> Added Trevor to the CC, who is the original author of --remote (see
> 06b1abb5b).
> 
> While I believe that adding such functionality makes perfect sense,
> I do not find it terribly obvious that setting the branch to '@' will
> make --remote skip this submodule. I wouldn't care so much if we'd
> only use this value internally, but this is user visible (and has to
> be set by the user if she wants to skip a submodule in --remote).
> 
> Setting the branch to an empty value feels a bit more natural, but
> I'm not so sure our config handling supports that well (we seem to
> assume in quite some places that empty equals unset). So I tend to
> prefer a new option for that.

I started out that way, thinking that would solve my initial problem of
having 4 submodules which should follow branch 'develop' and one
submodule that should stay put until explicitly changed. Instead having
empty an branch name gave me 'master' on that module, which was way off
current status of the code to say the least. This was all executed by
Jenkins which either gives you "update remote branches" or "not update
remote branches". Thus no other choice than to change the code.

The current 'git submodule add' takes option '-b' as branch and last
time I tried providing '@' as branch name it did not work. That
indicates that there should be a '-p' option for pinning the submodule
from the start. Having to fiddle with the .gitmodules file after the
fact is not really user friendly (though it have worked for me for a
while). I will have a go at an explicit setting in a week or two and get
back for comments on that. Perhaps some maintainers can give some hints
on adding a new option etc meanwhile. How about option '-p, ---pinned'
and setting 'submodule..pinned = true|false'?

Cleaned-up the test code according to Eric's comments, looks a bit more
stylish actually :).

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Questions about git-push for huge repositories

[Levin Du]

I consider 'git push' need further optimization.

Take kernel source code for example:

# Clone the kernel to A and B
$ git --version
git version 2.3.2
$ git clone --bare  ../kernel/ A
$ git clone --bare  ../kernel/ B

# Create the orphan commit and check
$ cd A
$ git branch test
Switched to a new branch 'test'
$ git replace --graft test
$ git rev-parse test
cbbae6741c60c9e09f87521e3a79810abd6a2fda
$ git rev-parse test^{tree}
929bdce0b48ca6079ad281a9d8ba24de3e49881a
$ git rev-parse replace/cbbae6741c60c9e09f87521e3a79810abd6a2fda
82d3e9ce1ca062c219f1209c5291ccd5603e5302
$ git rev-parse 82d3e9ce1ca062c219f1209c5291ccd5603e5302^{tree}
929bdce0b48ca6079ad281a9d8ba24de3e49881a
$ git log --pretty=oneline 82d3e9ce1ca062c219f1209c5291ccd5603e5302 | wc -l
1

We can see that commit 82d3e9ce1ca062c219f1209c5291ccd5603e5302 (root commit)
is meant to replace for commit cbbae6741c60c9e09f87521e3a79810abd6a2fda .
They both contain the same tree 929bdce0b48ca6079ad281a9d8ba24de3e49881a .

$ du -hs ../B
1.6G ../B
$ git push ../B 'refs/replace/*'
Counting objects: 51216, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (48963/48963), done.
Writing objects: 100% (51216/51216), 139.61 MiB | 17.88 MiB/s, done.
Total 51216 (delta 3647), reused 34580 (delta 1641)
To ../B
* [new branch]
refs/replace/cbbae6741c60c9e09f87521e3a79810abd6a2fda ->
refs/replace/cbbae6741c60c9e09f87521e3a79810abd6a2fda
$ du -hs ../B
1.7G ../B

It takes some time for 'git push' to compress the objects and B has
finally increased 0.1G,
which is for the newly commit whose tree is already in the repository.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/5] git-p4: add support for large file systems

[Luke Diamand]

On 7 September 2015 at 13:21,   wrote:
> From: Lars Schneider 
>
> One thing I don't like about the current implementation is that I don't see a
> way to test the "git-p4.pushLargeFiles" config. I could start a git lfs server
> locally but that seems a bit too much, no?

Perhaps add a trivial large-file derived class that just does enough
to show that it's all working. Then the final bit of pushing the large
files to LFS would remain forever untested, but it would be small
enough and simple enough not to be a real problem. If it ever broke,
it would indicate that either the trivial test class was a bit too
trivial, or that the abstraction was not abstract enough and there's
too much common-code in the derived classes.

Luke
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] filter-branch: add passed/remaining seconds on progress

[Eric Sunshine]

On Mon, Sep 7, 2015 at 9:52 AM, Gábor Bernát  wrote:
> From: Gabor Bernat 
>
> adds seconds progress and estimated seconds time if getting the current
> timestamp is supported by the date +%s command
>
> Signed-off-by: Gabor Bernat 
> ---
> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index 5b3f63d..565144a 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -275,11 +275,41 @@ commits=$(wc -l <../revs | tr -d " ")
>  test $commits -eq 0 && die "Found nothing to rewrite"
>
>  # Rewrite the commits
> +report_progress ()
> +{
> +if test -n "$progress"
> +then

Indent code within the function...

> +   if test $git_filter_branch__commit_count -gt $next_sample_at
> +   then
> +   now_timestamp=$(date +%s)
> +   elapsed_seconds=$(($now_timestamp - $start_timestamp))
> +   remaining_second=$(( ($commits - 
> $git_filter_branch__commit_count) * $elapsed_seconds / 
> $git_filter_branch__commit_count ))
> +   if test $elapsed_seconds -gt 0
> +   then
> +   next_sample_at=$(( ($elapsed_seconds + 1) * 
> $git_filter_branch__commit_count / $elapsed_seconds ))
> +   else
> +   next_sample_at=$(($next_sample_at + 1))
> +   fi
> +   progress=" ($elapsed_seconds seconds passed, remaining 
> $remaining_second predicted)"
> +   fi
> +fi
> +printf "\rRewrite $commit 
> ($git_filter_branch__commit_count/$commits)$progress"

The "\r" causes this status line to be overwritten each time through,
and since the processed commit count always increases, we know that
the original (without ETA) will never leave junk at the end of the
line. However, with estimated seconds also being displayed, does this
still hold? While it's true that elapsed seconds will increase,
estimated seconds may jump around, requiring different numbers of
digits to display. This may leave "garbage" digits at the end of line
from previous iterations, can't it?

> +}
>
>  git_filter_branch__commit_count=0
> +
> +progress= start_timestamp=
> +if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'
> +then
> +   next_sample_at=0
> +   progress="dummy to ensure this is not empty"
> +   start_timestamp=$(date '+%s')
> +fi
> +
>  while read commit parents; do
> 
> git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))
> -   printf "\rRewrite $commit ($git_filter_branch__commit_count/$commits)"
> +
> +   report_progress
>
> case "$filter_subdir" in
> "")
> --
> 2.6.0.rc0.3.gb3280a4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] interpret-trailers: allow running outside a repository

[Christian Couder]

On Sat, Sep 5, 2015 at 3:39 PM, John Keeping  wrote:
> It may be useful to run git-interpret-trailers without needing to be in
> a repository.

Yeah, it looks like it works fine outside a repo.

Tested-by: Christian Couder 

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC] Pinning of submodules

[Jens Lehmann]

Am 07.09.2015 um 01:43 schrieb Eric Sunshine:

On Sun, Sep 6, 2015 at 6:08 PM, Anders Ro  wrote:

On 04/09/15 07:02, Eric Sunshine wrote:

On Wed, Sep 2, 2015 at 7:34 PM, Anders Ro  wrote:

git-submodule.sh: pin submodule when branch name is '@'

Setting branch name to '@' for a submodule will disable 'git submodule
update --remote' calls for that specific submodule. I.e. instead of
follow the unspecified default choice of master, nothing is being
updated. This is useful when multiple submodules exist but not all
should follow the remote branch head.


With the disclaimer that I'm not a submodule user (to which the
answer might be obvious): What benefit is there in using a magic
value like this ("@") over, say, an explicit configuration setting?


 From what I have understood (not a submodule expert yet) the '@' is an
invalid branch name and should therefore not collide with any current
branches. My idea was to disable the '--remote' option when the user
have explicitly set an invalid branch name to not modify any current
behaviour. Though having an explicit option is of course more
clarifying. The current behaviour though is that empty branch name means
"follow master" which is somewhat unintuitive.


My concern in asking was that some future person might come up with
another scenario which also wants to use a "magic value" and would
have to invent / arrive at another "illegal" representation. Hence, an
explicit setting might be more appropriate. However, as stated, I
don't even use submodules, so I may be far off the mark. I've cc'd a
few of the submodule maintainers with the hope that they will chime
in.


Added Trevor to the CC, who is the original author of --remote (see
06b1abb5b).

While I believe that adding such functionality makes perfect sense,
I do not find it terribly obvious that setting the branch to '@' will
make --remote skip this submodule. I wouldn't care so much if we'd
only use this value internally, but this is user visible (and has to
be set by the user if she wants to skip a submodule in --remote).

Setting the branch to an empty value feels a bit more natural, but
I'm not so sure our config handling supports that well (we seem to
assume in quite some places that empty equals unset). So I tend to
prefer a new option for that.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 1/2] git-p4: print stderr if P4 read_pipe operation fails

[larsxschneider]

From: Lars Schneider 

If read_pipe crashes then the caller can inspect the error and handle
it appropriately.

Signed-off-by: Lars Schneider 
---
 git-p4.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 073f87b..36a4bcb 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -134,11 +134,11 @@ def read_pipe(c, ignore_error=False):
 sys.stderr.write('Reading pipe: %s\n' % str(c))
 
 expand = isinstance(c,basestring)
-p = subprocess.Popen(c, stdout=subprocess.PIPE, shell=expand)
+p = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.PIPE, 
shell=expand)
 pipe = p.stdout
 val = pipe.read()
 if p.wait() and not ignore_error:
-die('Command failed: %s' % str(c))
+die('Command failed: %s\nError: %s' % (str(c), p.stderr.read()))
 
 return val
 
-- 
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: More builtin git-am issues..

[Christian Couder]

On Sat, Sep 5, 2015 at 9:39 PM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> To salvage "interpret-trailers" needs a lot more, as we are
>> realizing that the definition that led to its external design does
>> not match the way users use footers in the real world.  This affects
>> the internal data representation and the whole thing needs to be
>> rethought.
>
> Note that I am not saying that you personally did any bad job while
> working on the interpret-trailers topic.  We collectively designed
> its feature based on a much narrower definition of what the trailer
> block is than what is used in the real world in practice, so we do
> not have a good way to locate an existing entry that is not a (key,
> value), no matter what the syntax to denote which is key and which
> is value is, insert a new entry that is not a (key, value), or
> remove an existing entry that is not a (key, value), all of which
> will be necessary to mutate trailer blocks people use in the real
> life.
>
> I think a good way forward would be to go like this:
>
>  * a helper function that starts from a flat  (or a
>strbuf) and identifies the end of the body of the message,
>i.e. find "^---$", "^Conflicts:$", etc. and skip blank lines
>backwards.  That is what ignore_non_trailer() in commit.c does,
>and that can be shared across everybody that mutates a log
>message.
>
>  * a helper function that takes the result of the above as a flat
> (or a strbuf) and identifies the beginning of a
>trailer block.  That may be just the matter of scanning backwards
>from the end of the trailer block ignore_non_trailer() identified
>for the first blank line, as I agree with Linus's "So quite
>frankly, at that point if git doesn't recognize it as a sign-off
>block, I don't think it's a big deal" in the thread.
>
>Not having that and not calling that function can reintroduce the
>recent "interpret-trailers corner case" bug Matthieu brought up.
>
> With these, everybody except interpret-trailers that mutates a log
> message can add a new signoff consistently.  And then, building on
> these, "interpret-trailers" can be written like this:
>
>  (1) starting from a flat  (or a strbuf), using the above
>  helpers, identify the parts of the log message that is the
>  trailer block (and you will know what is before and what is
>  after the trailer block).
>
>  (2) keep the part before the trailer block and the part after the
>  trailer block (this could be empty) in one strbuf each; we do
>  not want to mutate these parts, and it is pointless to split
>  them further into individual lines.
>
>  (3) express the lines in the trailer block in a richer data
>  structure that is easier to manipulate (i.e. reorder the lines,
>  insert, delete, etc.) and work on it.
>
>  (4) when manipulation of the trailer block is finished, reconstruct
>  the resulting message by concatenating the "before trailer"
>  part, "trailer" part, and "after trailer" part.
>
> As to the exact design of "a richer data structure" and the
> manipulation we may want on the trailer, I currently do not have a
> strong "it should be this way" opinion yet, but after looking at
> various examples Linus gave us in the discussion, my gut feelig is
> that it would be best to keep the operation simple and generic,
> e.g. "find a line that matches this regexp and replace it with this
> line", "insert this line at the end", "delete all lines that match
> this regexp", etc.

I will see what I can do about this.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v16 06/14] ref-filter: implement an `align` atom

[Karthik Nayak]

On Mon, Sep 7, 2015 at 1:40 AM, Eric Sunshine  wrote:
> On Sat, Sep 5, 2015 at 2:52 PM, Karthik Nayak  wrote:
>> Implement an `align` atom which left-, middle-, or right-aligns the
>> content between %(align:...) and %(end).
>>
>> It is followed by `:,`, where the `` is
>> either left, right or middle and `` is the size of the area
>> into which the content will be placed. If the content between
>> %(align:...) and %(end) is more than the width then no alignment is
>> performed. e.g. to align a refname atom to the middle with a total
>> width of 40 we can do: --format="%(align:middle,40)%(refname)%(end)".
>>
>> We introduce an `at_end` function for each element of the stack which
>> is to be called when the `end` atom is encountered. Using this we
>> implement end_align_handler() for the `align` atom, this aligns the
>> final strbuf by calling `strbuf_utf8_align()` from utf8.c.
>>
>> Ensure that quote formatting is performed on the whole of
>> %(align:...)...%(end) rather than individual atoms inside. We skip
>> quote formatting for individual atoms when the current stack element
>> is handling an %(align:...) atom and perform quote formatting at the
>> end when we encounter the %(end) atom of the second element of then
>> stack.
>>
>> Add documentation and tests for the same.
>>
>> Signed-off-by: Karthik Nayak 
>> ---
>> diff --git a/Documentation/git-for-each-ref.txt 
>> b/Documentation/git-for-each-ref.txt
>> index e49d578..b23412d 100644
>> --- a/Documentation/git-for-each-ref.txt
>> +++ b/Documentation/git-for-each-ref.txt
>> @@ -127,6 +127,16 @@ color::
>> +align::
>> +   Left-, middle-, or right-align the content between %(align:...)
>> +   and %(end). Followed by `:,`, where the
>> +   `` is either left, right or middle and `` is
>> +   the total length of the content with alignment. If the
>
> This should mention that  is optional and default to "left"
> if omitted.
>

Will add that.

>> +   contents length is more than the width then no alignment is
>> +   performed. If used with '--quote' everything in between
>> +   %(align:...) and %(end) is quoted, but if nested then only the
>> +   topmost level performs quoting.
>> diff --git a/ref-filter.c b/ref-filter.c
>> index e99c342..6c9ef08 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -773,6 +837,50 @@ static void populate_value(struct ref_array_item *ref)
>> else
>> v->s = " ";
>> continue;
>> +   } else if (match_atom_name(name, "align", )) {
>> +   struct align *align = >u.align;
>> +   struct strbuf **s;
>> +
>> +   if (!valp)
>> +   die(_("expected format: %%(align:, 
>> )"));
>
> I'm pretty sure this parsing code won't deal well with a space after
> the comma, so the space should be dropped from the diagnostic message
> advising the user of the correct format: s/, /,/
>

Will change.

>> +   /*
>> +* TODO: Implement a function similar to 
>> strbuf_split_str()
>> +* which would strip the terminator at the end.
>
> "...which would omit the separator from the end of each value."
>

Will change.

>> +*/
>> +   s = strbuf_split_str(valp, ',', 0);
>> +
>> +   /* If the position is given trim the ',' from the 
>> first strbuf */
>> +   if (s[1])
>> +   strbuf_setlen(s[0], s[0]->len - 1);
>> +   if (s[2])
>> +   die(_("align:, followed by 
>> garbage: %s"), s[2]->buf);
>
> If  is omitted, strbuf_split_str("42", ...) will return the
> array ["42", NULL] which won't have an element [2], which means this
> code will access beyond end-of-array. (This is another good argument
> for looping over s[] as Junio suggested rather than assuming these
> fixed yet optional positions. It can be hard to get it right.)

You're right, probably something on the lines of what Junio suggested here
http://article.gmane.org/gmane.comp.version-control.git/277041


-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html