Re: Advice needed for basic setup for home user

[Christian Couder]

On Tue, Aug 22, 2017 at 10:11 PM, Harry Putnam  wrote:
>
> I run 5-10 vbox vms' on this host with various OS's involved.
> With each host, I've kept a local repo of some key OS rc files.
> and a couple of hundred home made scripts.
>
> They all follow the same pattern of setup, but over time each repo
> becomes different from its cousins.
>
> I've never taken the step of centralizing the assorted local git repos
> into a central repo that keeps a branch or directory or whatever it
> would be called of each local repo.

You should probably decide first how you want the local git repos to be merged.
The result would be quite different if you have a branch or if you
have a directory for each local repo.

Maybe tutorials or books can help you get more familiar with Git so
that you can decide based on what would be best for your use case.

My wild guess would be that a branch in the central repo for each of
the "master" branches of the local repos would be the way to go. But I
don't know your use case much and cannot suggest you to do that based
on a wild guess.

> So that all the local repos would become a checked out module from the
> central git repo.

Git doesn't know about "modules". It has "submodules" but this is yet
another different thing and I am not sure at all that it would help in
your case.

> Or at any rate, something along that line... not even sure how I would
> set that up with git, but would like some overall advice about how to
> do that. A step thru or an outline would be very useful.

As it is not clear what end result you want, it is difficult to help you.

Re: "Your branch is up-to-date ..." is incorrect English grammar

[STEVEN WHITE]

Thank you, Martin and Junio. I'd be happy to review any patches. Thank
you both for your help! :)

(incidentally, I've had a devil of a time sending my two emails on
this topic--a total of 5 bounced mails for various reasons: didn't
like my email format, didn't like my email address, mail client, I've
had 2 syntax errors! I've only had success by sending via Gmail,
plain-text, in a browser; and God only knows whether I'm wasting my
time typing this. I'm not used to this world, so the bar is
sufficiently high and irritating that I'm very disinclined to dip my
toe into these waters again, which is a shame).

On Mon, Aug 21, 2017 at 3:37 PM, Junio C Hamano  wrote:
> Martin Ågren  writes:
>
>>> But “your branch is up-to-date” is INCORRECT. And, because it’s
>>> incorrect, it conveys an odd and unsettling experience to native
>>> English speakers whenever they read it.
>>>
>>> If you’re curious, you can find plenty of discussion of this point of
>>> grammar. Here’s just one example:
>>> https://english.stackexchange.com/questions/180611/do-i-keep-myself-up-to-date-or-up-to-date-on-something.
>>
>> There is also some previous discussion on this very list:
>> https://public-inbox.org/git/calftnmerxgetucvbo8zmvkcr302vq2s4htpohxae5nefmjt...@mail.gmail.com/T/#u
>>
>> The code base contains a few instances of "up-to-date" and "up to date".
>> A tree wide sweep could be made to update user-visible strings in the
>> code and in the documentation. Fixing source code comments seems like
>> overkill.
>
> It should be safe to update any message that is meant for human
> consumption (i.e. those inside the _("... message ...")) i18n
> marker).  As the use of "up-to-date" dates back to the days when
> Linus was still doing much code for our project, I suspect there may
> be some plumbing message that contains the phrase that scripts
> expect to stay spelled that way, and it is not OK to "fix" them.
>
> Thanks.

Re: [PATCH] vcs-svn/repo_tree.h: remove repo_init declaration

[Junio C Hamano]

Jonathan Nieder  writes:

> Junio C Hamano wrote:
> ...
>> It looks to me that this is a reduced duplicate of what brian posted
>> yesterday.  The first two patches in the 6-patch series that you
>> commented on, I think, covers what this change wants to achieve and
>> probably a lot more.  I've merged those two already to 'next' and
>> was about to push the result out.
>
> I just sent
> https://public-inbox.org/git/20170822233732.gx13...@aiede.mtv.corp.google.com/
> to fix this more thoroughly.  I can rebase on top of bc/vcs-svn-cleanup
> if that is helpful --- just say the word.

Yup, as I merged those two preliminary clean-up separately to 'next'
already, it would be more efficient to build on top.  I do not know
what you are planning with the other patches (I only saw Stefan's
one, not your series) so do not yet have suggestion on how to work
well with Brian's series that builds on his two clean-up patches.
Hopefully they are orthogonal?

Thanks.

Re: What's cooking in git.git (Aug 2017, #05; Tue, 22)

[Brandon Williams]

On 08/22, Junio C Hamano wrote:
> 
> * bw/submodule-config-cleanup (2017-08-03) 17 commits
>  - submodule: remove gitmodules_config
>  - unpack-trees: improve loading of .gitmodules
>  - submodule-config: lazy-load a repository's .gitmodules file
>  - submodule-config: move submodule-config functions to submodule-config.c
>  - submodule-config: remove support for overlaying repository config
>  - diff: stop allowing diff to have submodules configured in .git/config
>  - submodule: remove submodule_config callback routine
>  - unpack-trees: don't respect submodule.update
>  - submodule: don't rely on overlayed config when setting diffopts
>  - fetch: don't overlay config with submodule-config
>  - submodule--helper: don't overlay config in update-clone
>  - submodule--helper: don't overlay config in remote_submodule_branch
>  - add, reset: ensure submodules can be added or reset
>  - submodule: don't use submodule_from_name
>  - t7411: check configuration parsing errors
>  - Merge branch 'bc/object-id' into bw/submodule-config-cleanup
>  - Merge branch 'bw/grep-recurse-submodules' into bw/submodule-config-cleanup
> 
>  Code clean-up to avoid mixing values read from the .gitmodules file
>  and values read from the .git/config file.
> 
>  So, after the recent discussion around submodule..update (and
>  its resolution "use submodule..active; "reset --hard" must
>  ignore the .update thing"), this is now good to go, I presume?
>  Please yell at me that I am clueless if that is not the case ;-)

Yep I came to the same conclusion that you did so this should be good to
go!

-- 
Brandon Williams

[PATCH v2] Doc: clarify that pack-objects makes packs, plural

[Jonathan Tan]

The documentation for pack-objects describes that it creates "a packed
archive of objects", which is confusing because it may create multiple
packs if --max-pack-size is set. Update the documentation to clarify
this, and explaining in which cases such a feature would be useful.

Signed-off-by: Jonathan Tan 
---
Thanks. I've reverted the NAME change and used some of your suggestion
for the --max-pack-size documentation.
---
 Documentation/git-pack-objects.txt | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-pack-objects.txt 
b/Documentation/git-pack-objects.txt
index 8973510a4..473a16135 100644
--- a/Documentation/git-pack-objects.txt
+++ b/Documentation/git-pack-objects.txt
@@ -18,8 +18,9 @@ SYNOPSIS
 
 DESCRIPTION
 ---
-Reads list of objects from the standard input, and writes a packed
-archive with specified base-name, or to the standard output.
+Reads list of objects from the standard input, and writes either one or
+more packed archives with the specified base-name to disk, or a packed
+archive to the standard output.
 
 A packed archive is an efficient way to transfer a set of objects
 between two repositories as well as an access efficient archival
@@ -47,9 +48,9 @@ transport by their peers.
 OPTIONS
 ---
 base-name::
-   Write into a pair of files (.pack and .idx), using
+   Write into pairs of files (.pack and .idx), using
 to determine the name of the created file.
-   When this option is used, the two files are written in
+   When this option is used, the two files in a pair are written in
-.{pack,idx} files.   is a hash
based on the pack content and is written to the standard
output of the command.
@@ -108,9 +109,13 @@ base-name::
is taken from the `pack.windowMemory` configuration variable.
 
 --max-pack-size=::
-   Maximum size of each output pack file. The size can be suffixed with
+   In unusual scenarios, you may not be able to create files
+   larger than a certain size on your filesystem, and this option
+   can be used to tell the command to split the output packfile
+   into multiple independent packfiles, each not larger than the
+   given size. The size can be suffixed with
"k", "m", or "g". The minimum size allowed is limited to 1 MiB.
-   If specified, multiple packfiles may be created, which also
+   This option
prevents the creation of a bitmap index.
The default is unlimited, unless the config variable
`pack.packSizeLimit` is set.
-- 
2.14.1.342.g6490525c54-goog

[PATCH v2 4/4] vcs-svn: move remaining repo_tree functions to fast_export.h

[Jonathan Nieder]

These used to be for manipulating the in-memory repo_tree structure,
but nowadays they are convenience wrappers to handle a few git-vs-svn
mismatches:

 1. Git does not track empty directories but Subversion does.  When
looking up a path in git that Subversion thinks exists and finding
nothing, we can safely assume that the path represents a
directory.  This is needed when a later Subversion revision
modifies that directory.

 2. Subversion allows deleting a file by copying.  In Git fast-import
we have to handle that more explicitly as a deletion.

These are details of the tool's interaction with git fast-import.
Move them to fast_export.c, where other such details are handled.

This way the function names do not start with a repo_ prefix that
would clash with the repository object introduced in
v2.14.0-rc0~38^2~16 (repository: introduce the repository object,
2017-06-22) or an svn_ prefix that would clash with libsvn (in case
someone wants to link this code with libsvn some day).

Signed-off-by: Jonathan Nieder 
---
The only change is the commit message.  These functions are already
namespaced on the bc/vcs-svn-cleanup, so added a note about that.

That's the end of the series.  Thanks for reading.

 Makefile  |  1 -
 vcs-svn/fast_export.c | 35 ++-
 vcs-svn/fast_export.h |  3 +++
 vcs-svn/repo_tree.c   | 43 ---
 vcs-svn/repo_tree.h   |  7 ---
 vcs-svn/svndump.c |  5 ++---
 6 files changed, 39 insertions(+), 55 deletions(-)
 delete mode 100644 vcs-svn/repo_tree.c
 delete mode 100644 vcs-svn/repo_tree.h

diff --git a/Makefile b/Makefile
index 46ad908ec5..9b00d5b219 100644
--- a/Makefile
+++ b/Makefile
@@ -1942,7 +1942,6 @@ XDIFF_OBJS += xdiff/xhistogram.o
 
 VCSSVN_OBJS += vcs-svn/line_buffer.o
 VCSSVN_OBJS += vcs-svn/sliding_window.o
-VCSSVN_OBJS += vcs-svn/repo_tree.o
 VCSSVN_OBJS += vcs-svn/fast_export.o
 VCSSVN_OBJS += vcs-svn/svndiff.o
 VCSSVN_OBJS += vcs-svn/svndump.o
diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 6d133ed6bc..5bd455b8c8 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -6,7 +6,6 @@
 #include "cache.h"
 #include "quote.h"
 #include "fast_export.h"
-#include "repo_tree.h"
 #include "strbuf.h"
 #include "svndiff.h"
 #include "sliding_window.h"
@@ -312,6 +311,40 @@ int fast_export_ls(const char *path, uint32_t *mode, 
struct strbuf *dataref)
return parse_ls_response(get_response_line(), mode, dataref);
 }
 
+const char *fast_export_read_path(const char *path, uint32_t *mode_out)
+{
+   int err;
+   static struct strbuf buf = STRBUF_INIT;
+
+   strbuf_reset();
+   err = fast_export_ls(path, mode_out, );
+   if (err) {
+   if (errno != ENOENT)
+   die_errno("BUG: unexpected fast_export_ls error");
+   /* Treat missing paths as directories. */
+   *mode_out = S_IFDIR;
+   return NULL;
+   }
+   return buf.buf;
+}
+
+void fast_export_copy(uint32_t revision, const char *src, const char *dst)
+{
+   int err;
+   uint32_t mode;
+   static struct strbuf data = STRBUF_INIT;
+
+   strbuf_reset();
+   err = fast_export_ls_rev(revision, src, , );
+   if (err) {
+   if (errno != ENOENT)
+   die_errno("BUG: unexpected fast_export_ls_rev error");
+   fast_export_delete(dst);
+   return;
+   }
+   fast_export_modify(dst, mode, data.buf);
+}
+
 void fast_export_blob_delta(uint32_t mode,
uint32_t old_mode, const char *old_data,
off_t len, struct line_buffer *input)
diff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h
index c8b5adb811..ae8ab7e589 100644
--- a/vcs-svn/fast_export.h
+++ b/vcs-svn/fast_export.h
@@ -28,4 +28,7 @@ int fast_export_ls_rev(uint32_t rev, const char *path,
 int fast_export_ls(const char *path,
uint32_t *mode_out, struct strbuf *dataref_out);
 
+void fast_export_copy(uint32_t revision, const char *src, const char *dst);
+const char *fast_export_read_path(const char *path, uint32_t *mode_out);
+
 #endif
diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c
deleted file mode 100644
index 5bd4977cb6..00
--- a/vcs-svn/repo_tree.c
+++ /dev/null
@@ -1,43 +0,0 @@
-/*
- * Licensed under a two-clause BSD-style license.
- * See LICENSE for details.
- */
-
-#include "git-compat-util.h"
-#include "strbuf.h"
-#include "repo_tree.h"
-#include "fast_export.h"
-
-const char *svn_repo_read_path(const char *path, uint32_t *mode_out)
-{
-   int err;
-   static struct strbuf buf = STRBUF_INIT;
-
-   strbuf_reset();
-   err = fast_export_ls(path, mode_out, );
-   if (err) {
-   if (errno != ENOENT)
-   die_errno("BUG: unexpected fast_export_ls error");
-   /* Treat missing paths as 

[PATCH v2 3/4] vcs-svn: remove repo_delete wrapper function

[Jonathan Nieder]

Since v1.7.10-rc0~118^2~4^2~4^2~3 (vcs-svn: pass paths through to
fast-import, 2010-12-13) this is an alias for fast_export_delete.
Remove the unnecessary layer of indirection.

No functional change intended.

Signed-off-by: Jonathan Nieder 
---
Unchanged.

 vcs-svn/repo_tree.c | 5 -
 vcs-svn/repo_tree.h | 1 -
 vcs-svn/svndump.c   | 4 ++--
 3 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c
index 1a6f32d7cb..5bd4977cb6 100644
--- a/vcs-svn/repo_tree.c
+++ b/vcs-svn/repo_tree.c
@@ -41,8 +41,3 @@ void svn_repo_copy(uint32_t revision, const char *src, const 
char *dst)
}
fast_export_modify(dst, mode, data.buf);
 }
-
-void svn_repo_delete(const char *path)
-{
-   fast_export_delete(path);
-}
diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
index c840bc9bae..0cd2761183 100644
--- a/vcs-svn/repo_tree.h
+++ b/vcs-svn/repo_tree.h
@@ -3,6 +3,5 @@
 
 void svn_repo_copy(uint32_t revision, const char *src, const char *dst);
 const char *svn_repo_read_path(const char *path, uint32_t *mode_out);
-void svn_repo_delete(const char *path);
 
 #endif
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index c0fa4eb723..41113119bd 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -225,11 +225,11 @@ static void handle_node(void)
if (have_text || have_props || node_ctx.srcRev)
die("invalid dump: deletion node has "
"copyfrom info, text, or properties");
-   svn_repo_delete(node_ctx.dst.buf);
+   fast_export_delete(node_ctx.dst.buf);
return;
}
if (node_ctx.action == NODEACT_REPLACE) {
-   svn_repo_delete(node_ctx.dst.buf);
+   fast_export_delete(node_ctx.dst.buf);
node_ctx.action = NODEACT_ADD;
}
if (node_ctx.srcRev) {
-- 
2.14.1.342.g6490525c54

[PATCH v2 2/4] vcs-svn: remove custom mode constants

[Jonathan Nieder]

In the rest of Git, these modes are spelled as S_IFDIR,
S_IFREG | 0644, S_IFREG | 0755, and S_IFLNK.  Use the same constants
in svn-fe for simplicity and consistency.

No functional change intended.

Signed-off-by: Jonathan Nieder 
---
Unchanged.

 vcs-svn/fast_export.c |  6 +++---
 vcs-svn/repo_tree.c   |  2 +-
 vcs-svn/repo_tree.h   |  5 -
 vcs-svn/svndump.c | 24 
 4 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 97cba39cdf..6d133ed6bc 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -210,7 +210,7 @@ static long apply_delta(off_t len, struct line_buffer 
*input,
die("invalid cat-blob response: %s", response);
check_preimage_overflow(preimage.max_off, 1);
}
-   if (old_mode == REPO_MODE_LNK) {
+   if (old_mode == S_IFLNK) {
strbuf_addstr(, "link ");
check_preimage_overflow(preimage.max_off, strlen("link "));
preimage.max_off += strlen("link ");
@@ -244,7 +244,7 @@ void fast_export_buf_to_data(const struct strbuf *data)
 void fast_export_data(uint32_t mode, off_t len, struct line_buffer *input)
 {
assert(len >= 0);
-   if (mode == REPO_MODE_LNK) {
+   if (mode == S_IFLNK) {
/* svn symlink blobs start with "link " */
if (len < 5)
die("invalid dump: symlink too short for \"link\" 
prefix");
@@ -320,7 +320,7 @@ void fast_export_blob_delta(uint32_t mode,
 
assert(len >= 0);
postimage_len = apply_delta(len, input, old_data, old_mode);
-   if (mode == REPO_MODE_LNK) {
+   if (mode == S_IFLNK) {
buffer_skip_bytes(, strlen("link "));
postimage_len -= strlen("link ");
}
diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c
index d77cb0ada7..1a6f32d7cb 100644
--- a/vcs-svn/repo_tree.c
+++ b/vcs-svn/repo_tree.c
@@ -19,7 +19,7 @@ const char *svn_repo_read_path(const char *path, uint32_t 
*mode_out)
if (errno != ENOENT)
die_errno("BUG: unexpected fast_export_ls error");
/* Treat missing paths as directories. */
-   *mode_out = REPO_MODE_DIR;
+   *mode_out = S_IFDIR;
return NULL;
}
return buf.buf;
diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
index 0d3bbb677d..c840bc9bae 100644
--- a/vcs-svn/repo_tree.h
+++ b/vcs-svn/repo_tree.h
@@ -1,11 +1,6 @@
 #ifndef REPO_TREE_H_
 #define REPO_TREE_H_
 
-#define REPO_MODE_DIR 004
-#define REPO_MODE_BLB 0100644
-#define REPO_MODE_EXE 0100755
-#define REPO_MODE_LNK 012
-
 void svn_repo_copy(uint32_t revision, const char *src, const char *dst);
 const char *svn_repo_read_path(const char *path, uint32_t *mode_out);
 void svn_repo_delete(const char *path);
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 7da84b2aab..c0fa4eb723 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -134,13 +134,13 @@ static void handle_property(const struct strbuf *key_buf,
die("invalid dump: sets type twice");
}
if (!val) {
-   node_ctx.type = REPO_MODE_BLB;
+   node_ctx.type = S_IFREG | 0644;
return;
}
*type_set = 1;
node_ctx.type = keylen == strlen("svn:executable") ?
-   REPO_MODE_EXE :
-   REPO_MODE_LNK;
+   (S_IFREG | 0755) :
+   S_IFLNK;
}
 }
 
@@ -219,7 +219,7 @@ static void handle_node(void)
 */
static const char *const empty_blob = "::empty::";
const char *old_data = NULL;
-   uint32_t old_mode = REPO_MODE_BLB;
+   uint32_t old_mode = S_IFREG | 0644;
 
if (node_ctx.action == NODEACT_DELETE) {
if (have_text || have_props || node_ctx.srcRev)
@@ -237,27 +237,27 @@ static void handle_node(void)
if (node_ctx.action == NODEACT_ADD)
node_ctx.action = NODEACT_CHANGE;
}
-   if (have_text && type == REPO_MODE_DIR)
+   if (have_text && type == S_IFDIR)
die("invalid dump: directories cannot have text attached");
 
/*
 * Find old content (old_data) and decide on the new mode.
 */
if (node_ctx.action == NODEACT_CHANGE && !*node_ctx.dst.buf) {
-   if (type != REPO_MODE_DIR)
+   if (type != S_IFDIR)
die("invalid dump: root of tree is not a regular file");
old_data = NULL;
} else if (node_ctx.action == NODEACT_CHANGE) {
uint32_t mode;
old_data = svn_repo_read_path(node_ctx.dst.buf, );
-   if (mode == REPO_MODE_DIR && type != REPO_MODE_DIR)
+  

[PATCH v2 1/4] vcs-svn: remove more unused prototypes and declarations

[Jonathan Nieder]

I forgot to remove these in v1.7.10-rc0~118^2~4^2~5^2~4 (vcs-svn:
eliminate repo_tree structure, 2010-12-10).

This finishes what was started in commit 36f63b50 (vcs-svn: remove
unused prototypes, 2017-08-21).

Signed-off-by: Jonathan Nieder 
---
 vcs-svn/repo_tree.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
index 555b64bbb6..0d3bbb677d 100644
--- a/vcs-svn/repo_tree.h
+++ b/vcs-svn/repo_tree.h
@@ -1,14 +1,11 @@
 #ifndef REPO_TREE_H_
 #define REPO_TREE_H_
 
-struct strbuf;
-
 #define REPO_MODE_DIR 004
 #define REPO_MODE_BLB 0100644
 #define REPO_MODE_EXE 0100755
 #define REPO_MODE_LNK 012
 
-uint32_t next_blob_mark(void);
 void svn_repo_copy(uint32_t revision, const char *src, const char *dst);
 const char *svn_repo_read_path(const char *path, uint32_t *mode_out);
 void svn_repo_delete(const char *path);
-- 
2.14.1.342.g6490525c54

[PATCH v2 bc/vcs-svn-cleanup] vcs-svn: remove repo_tree library

[Jonathan Nieder]

Hi again,

Jonathan Nieder wrote:

> This is an alternative to bc/vcs-svn-cleanup from 'next'.  Those
> patches weren't cc-ed to me and I missed them --- sorry about that.  I
> can rebase on top of them if that is more convenient.

Here is the same series rebased on top of bc/vcs-svn-cleanup.

Thoughts welcome, as always.

Jonathan Nieder (4):
  vcs-svn: remove more unused prototypes and declarations
  vcs-svn: remove custom mode constants
  vcs-svn: remove repo_delete wrapper function
  vcs-svn: move remaining repo_tree functions to fast_export.h

 Makefile  |  1 -
 vcs-svn/fast_export.c | 41 +
 vcs-svn/fast_export.h |  3 +++
 vcs-svn/repo_tree.c   | 48 
 vcs-svn/repo_tree.h   | 16 
 vcs-svn/svndump.c | 33 -
 6 files changed, 56 insertions(+), 86 deletions(-)
 delete mode 100644 vcs-svn/repo_tree.c
 delete mode 100644 vcs-svn/repo_tree.h

> [1] https://public-inbox.org/git/20170822213501.5928-1-sbel...@google.com
> [2] 
> https://public-inbox.org/git/2017082122.26729-3-sand...@crustytoothpaste.net

Re: [PATCH] vcs-svn/repo_tree.h: remove repo_init declaration

[Jonathan Nieder]

Junio C Hamano wrote:
> Stefan Beller  writes:

>> The svn specific declaration of repo_init was not used since 723b7a2789
>> (vcs-svn: eliminate repo_tree structure, 2010-12-10).
>>
>> This was noticed when including repository.h via cache.h as that has the
>> same function with a different signature.
>>
>> Helped-by: Jonathan Nieder 
>> Signed-off-by: Stefan Beller 
>> ---
>
> It looks to me that this is a reduced duplicate of what brian posted
> yesterday.  The first two patches in the 6-patch series that you
> commented on, I think, covers what this change wants to achieve and
> probably a lot more.  I've merged those two already to 'next' and
> was about to push the result out.

I just sent
https://public-inbox.org/git/20170822233732.gx13...@aiede.mtv.corp.google.com/
to fix this more thoroughly.  I can rebase on top of bc/vcs-svn-cleanup
if that is helpful --- just say the word.

Thanks,
Jonathan

Re: [PATCH 0/4] vcs-svn: remove repo_tree library

[Stefan Beller]

On Tue, Aug 22, 2017 at 4:37 PM, Jonathan Nieder  wrote:
> Hi,
>
> Stefan noticed that repo_init from vcs-svn/repo_tree.h conflicts with
> repository.h[1].  Earlier brian m. carlson noticed the same thing[2].
>
> Originally repo_tree.h was used to manage an in-memory representation
> of the state of the svn tree being imported.  When that in-memory
> representation was retired, we were lazy and left some utility
> functions there.  Here is a patch series to finish cleaning up and
> remove vcs-svn/repo_tree.h completely.
>
> This is an alternative to bc/vcs-svn-cleanup from 'next'.  Those
> patches weren't cc-ed to me and I missed them --- sorry about that.  I
> can rebase on top of them if that is more convenient.

A rebased version would be easier IIUC Junios reply to
the one-off that I sent.

The patches look good.

Thanks,
Stefan

>
> Thoughts of all kinds welcome, as always.
>
> Thanks,
> Jonathan
>
> Jonathan Nieder (4):
>   vcs-svn: remove prototypes for missing functions
>   vcs-svn: remove custom mode constants
>   vcs-svn: remove repo_delete wrapper function
>   vcs-svn: move remaining repo_tree functions to fast_export.h
>
>  Makefile  |  1 -
>  vcs-svn/fast_export.c | 41 +
>  vcs-svn/fast_export.h |  3 +++
>  vcs-svn/repo_tree.c   | 48 
>  vcs-svn/repo_tree.h   | 23 ---
>  vcs-svn/svndump.c | 33 -
>  6 files changed, 56 insertions(+), 93 deletions(-)
>  delete mode 100644 vcs-svn/repo_tree.c
>  delete mode 100644 vcs-svn/repo_tree.h
>
> [1] https://public-inbox.org/git/20170822213501.5928-1-sbel...@google.com
> [2] 
> https://public-inbox.org/git/2017082122.26729-3-sand...@crustytoothpaste.net

[PATCH 4/4] vcs-svn: move remaining repo_tree functions to fast_export.h

[Jonathan Nieder]

These used to be for manipulating the in-memory repo_tree structure,
but nowadays they are convenience wrappers to handle a few git-vs-svn
mismatches:

 1. Git does not track empty directories but Subversion does.  When
looking up a path in git that Subversion thinks exists and finding
nothing, we can safely assume that the path represents a
directory.  This is needed when a later Subversion revision
modifies that directory.

 2. Subversion allows deleting a file by copying.  In Git fast-import
we have to handle that more explicitly as a deletion.

These are details of the tool's interaction with git fast-import.
Move them to fast_export.c, where other such details are handled.

This way the functions do not start with a repo_ prefix that would
clash with the repository object introduced in v2.14.0-rc0~38^2~16
(repository: introduce the repository object, 2017-06-22).

Reported-by: brian m. carlson 
Signed-off-by: Jonathan Nieder 
---
 Makefile  |  1 -
 vcs-svn/fast_export.c | 35 ++-
 vcs-svn/fast_export.h |  3 +++
 vcs-svn/repo_tree.c   | 43 ---
 vcs-svn/repo_tree.h   |  7 ---
 vcs-svn/svndump.c |  5 ++---
 6 files changed, 39 insertions(+), 55 deletions(-)
 delete mode 100644 vcs-svn/repo_tree.c
 delete mode 100644 vcs-svn/repo_tree.h

diff --git a/Makefile b/Makefile
index 86ec29202b..493b8f5d2d 100644
--- a/Makefile
+++ b/Makefile
@@ -2038,7 +2038,6 @@ XDIFF_OBJS += xdiff/xhistogram.o
 
 VCSSVN_OBJS += vcs-svn/line_buffer.o
 VCSSVN_OBJS += vcs-svn/sliding_window.o
-VCSSVN_OBJS += vcs-svn/repo_tree.o
 VCSSVN_OBJS += vcs-svn/fast_export.o
 VCSSVN_OBJS += vcs-svn/svndiff.o
 VCSSVN_OBJS += vcs-svn/svndump.o
diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 7790f8e1d1..3fd047a8b8 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -6,7 +6,6 @@
 #include "cache.h"
 #include "quote.h"
 #include "fast_export.h"
-#include "repo_tree.h"
 #include "strbuf.h"
 #include "svndiff.h"
 #include "sliding_window.h"
@@ -312,6 +311,40 @@ int fast_export_ls(const char *path, uint32_t *mode, 
struct strbuf *dataref)
return parse_ls_response(get_response_line(), mode, dataref);
 }
 
+const char *fast_export_read_path(const char *path, uint32_t *mode_out)
+{
+   int err;
+   static struct strbuf buf = STRBUF_INIT;
+
+   strbuf_reset();
+   err = fast_export_ls(path, mode_out, );
+   if (err) {
+   if (errno != ENOENT)
+   die_errno("BUG: unexpected fast_export_ls error");
+   /* Treat missing paths as directories. */
+   *mode_out = S_IFDIR;
+   return NULL;
+   }
+   return buf.buf;
+}
+
+void fast_export_copy(uint32_t revision, const char *src, const char *dst)
+{
+   int err;
+   uint32_t mode;
+   static struct strbuf data = STRBUF_INIT;
+
+   strbuf_reset();
+   err = fast_export_ls_rev(revision, src, , );
+   if (err) {
+   if (errno != ENOENT)
+   die_errno("BUG: unexpected fast_export_ls_rev error");
+   fast_export_delete(dst);
+   return;
+   }
+   fast_export_modify(dst, mode, data.buf);
+}
+
 void fast_export_blob_delta(uint32_t mode,
uint32_t old_mode, const char *old_data,
off_t len, struct line_buffer *input)
diff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h
index b9a3b71c99..60b79c35b9 100644
--- a/vcs-svn/fast_export.h
+++ b/vcs-svn/fast_export.h
@@ -28,4 +28,7 @@ int fast_export_ls_rev(uint32_t rev, const char *path,
 int fast_export_ls(const char *path,
uint32_t *mode_out, struct strbuf *dataref_out);
 
+void fast_export_copy(uint32_t revision, const char *src, const char *dst);
+const char *fast_export_read_path(const char *path, uint32_t *mode_out);
+
 #endif
diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c
deleted file mode 100644
index 99747e373a..00
--- a/vcs-svn/repo_tree.c
+++ /dev/null
@@ -1,43 +0,0 @@
-/*
- * Licensed under a two-clause BSD-style license.
- * See LICENSE for details.
- */
-
-#include "git-compat-util.h"
-#include "strbuf.h"
-#include "repo_tree.h"
-#include "fast_export.h"
-
-const char *repo_read_path(const char *path, uint32_t *mode_out)
-{
-   int err;
-   static struct strbuf buf = STRBUF_INIT;
-
-   strbuf_reset();
-   err = fast_export_ls(path, mode_out, );
-   if (err) {
-   if (errno != ENOENT)
-   die_errno("BUG: unexpected fast_export_ls error");
-   /* Treat missing paths as directories. */
-   *mode_out = S_IFDIR;
-   return NULL;
-   }
-   return buf.buf;
-}
-
-void repo_copy(uint32_t revision, const char *src, const char *dst)
-{
-   int err;
-   uint32_t mode;
-   static 

[PATCH 3/4] vcs-svn: remove repo_delete wrapper function

[Jonathan Nieder]

Since v1.7.10-rc0~118^2~4^2~4^2~3 (vcs-svn: pass paths through to
fast-import, 2010-12-13) this is an alias for fast_export_delete.
Remove the unnecessary layer of indirection.

No functional change intended.

Signed-off-by: Jonathan Nieder 
---
 vcs-svn/repo_tree.c | 5 -
 vcs-svn/repo_tree.h | 1 -
 vcs-svn/svndump.c   | 4 ++--
 3 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c
index 9107f9663d..99747e373a 100644
--- a/vcs-svn/repo_tree.c
+++ b/vcs-svn/repo_tree.c
@@ -41,8 +41,3 @@ void repo_copy(uint32_t revision, const char *src, const char 
*dst)
}
fast_export_modify(dst, mode, data.buf);
 }
-
-void repo_delete(const char *path)
-{
-   fast_export_delete(path);
-}
diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
index 7f59fd9148..56a3209d01 100644
--- a/vcs-svn/repo_tree.h
+++ b/vcs-svn/repo_tree.h
@@ -3,6 +3,5 @@
 
 void repo_copy(uint32_t revision, const char *src, const char *dst);
 const char *repo_read_path(const char *path, uint32_t *mode_out);
-void repo_delete(const char *path);
 
 #endif
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index dc42ae3316..d51136fac5 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -225,11 +225,11 @@ static void handle_node(void)
if (have_text || have_props || node_ctx.srcRev)
die("invalid dump: deletion node has "
"copyfrom info, text, or properties");
-   repo_delete(node_ctx.dst.buf);
+   fast_export_delete(node_ctx.dst.buf);
return;
}
if (node_ctx.action == NODEACT_REPLACE) {
-   repo_delete(node_ctx.dst.buf);
+   fast_export_delete(node_ctx.dst.buf);
node_ctx.action = NODEACT_ADD;
}
if (node_ctx.srcRev) {
-- 
2.14.1.342.g6490525c54

[PATCH 2/4] vcs-svn: remove custom mode constants

[Jonathan Nieder]

In the rest of Git, these modes are spelled as S_IFDIR,
S_IFREG | 0644, S_IFREG | 0755, and S_IFLNK.  Use the same constants
in svn-fe for simplicity and consistency.

No functional change intended.

Signed-off-by: Jonathan Nieder 
---
 vcs-svn/fast_export.c |  6 +++---
 vcs-svn/repo_tree.c   |  2 +-
 vcs-svn/repo_tree.h   |  5 -
 vcs-svn/svndump.c | 24 
 4 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 5a89db30e3..7790f8e1d1 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -210,7 +210,7 @@ static long apply_delta(off_t len, struct line_buffer 
*input,
die("invalid cat-blob response: %s", response);
check_preimage_overflow(preimage.max_off, 1);
}
-   if (old_mode == REPO_MODE_LNK) {
+   if (old_mode == S_IFLNK) {
strbuf_addstr(, "link ");
check_preimage_overflow(preimage.max_off, strlen("link "));
preimage.max_off += strlen("link ");
@@ -244,7 +244,7 @@ void fast_export_buf_to_data(const struct strbuf *data)
 void fast_export_data(uint32_t mode, off_t len, struct line_buffer *input)
 {
assert(len >= 0);
-   if (mode == REPO_MODE_LNK) {
+   if (mode == S_IFLNK) {
/* svn symlink blobs start with "link " */
if (len < 5)
die("invalid dump: symlink too short for \"link\" 
prefix");
@@ -320,7 +320,7 @@ void fast_export_blob_delta(uint32_t mode,
 
assert(len >= 0);
postimage_len = apply_delta(len, input, old_data, old_mode);
-   if (mode == REPO_MODE_LNK) {
+   if (mode == S_IFLNK) {
buffer_skip_bytes(, strlen("link "));
postimage_len -= strlen("link ");
}
diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c
index 67d27f0b6c..9107f9663d 100644
--- a/vcs-svn/repo_tree.c
+++ b/vcs-svn/repo_tree.c
@@ -19,7 +19,7 @@ const char *repo_read_path(const char *path, uint32_t 
*mode_out)
if (errno != ENOENT)
die_errno("BUG: unexpected fast_export_ls error");
/* Treat missing paths as directories. */
-   *mode_out = REPO_MODE_DIR;
+   *mode_out = S_IFDIR;
return NULL;
}
return buf.buf;
diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
index 6c2f5f8a00..7f59fd9148 100644
--- a/vcs-svn/repo_tree.h
+++ b/vcs-svn/repo_tree.h
@@ -1,11 +1,6 @@
 #ifndef REPO_TREE_H_
 #define REPO_TREE_H_
 
-#define REPO_MODE_DIR 004
-#define REPO_MODE_BLB 0100644
-#define REPO_MODE_EXE 0100755
-#define REPO_MODE_LNK 012
-
 void repo_copy(uint32_t revision, const char *src, const char *dst);
 const char *repo_read_path(const char *path, uint32_t *mode_out);
 void repo_delete(const char *path);
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 1846685a21..dc42ae3316 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -134,13 +134,13 @@ static void handle_property(const struct strbuf *key_buf,
die("invalid dump: sets type twice");
}
if (!val) {
-   node_ctx.type = REPO_MODE_BLB;
+   node_ctx.type = S_IFREG | 0644;
return;
}
*type_set = 1;
node_ctx.type = keylen == strlen("svn:executable") ?
-   REPO_MODE_EXE :
-   REPO_MODE_LNK;
+   (S_IFREG | 0755) :
+   S_IFLNK;
}
 }
 
@@ -219,7 +219,7 @@ static void handle_node(void)
 */
static const char *const empty_blob = "::empty::";
const char *old_data = NULL;
-   uint32_t old_mode = REPO_MODE_BLB;
+   uint32_t old_mode = S_IFREG | 0644;
 
if (node_ctx.action == NODEACT_DELETE) {
if (have_text || have_props || node_ctx.srcRev)
@@ -237,27 +237,27 @@ static void handle_node(void)
if (node_ctx.action == NODEACT_ADD)
node_ctx.action = NODEACT_CHANGE;
}
-   if (have_text && type == REPO_MODE_DIR)
+   if (have_text && type == S_IFDIR)
die("invalid dump: directories cannot have text attached");
 
/*
 * Find old content (old_data) and decide on the new mode.
 */
if (node_ctx.action == NODEACT_CHANGE && !*node_ctx.dst.buf) {
-   if (type != REPO_MODE_DIR)
+   if (type != S_IFDIR)
die("invalid dump: root of tree is not a regular file");
old_data = NULL;
} else if (node_ctx.action == NODEACT_CHANGE) {
uint32_t mode;
old_data = repo_read_path(node_ctx.dst.buf, );
-   if (mode == REPO_MODE_DIR && type != REPO_MODE_DIR)
+   if (mode == S_IFDIR && 

[PATCH 1/4] vcs-svn: remove prototypes for missing functions

[Jonathan Nieder]

I forgot to remove these prototypes when removing these functions in
v1.7.10-rc0~118^2~4^2~5^2~4 (vcs-svn: eliminate repo_tree structure,
2010-12-10).

Noticed by building a new file that included both repo_tree.h and
repository.h ("error: conflicting types for 'repo_init'").

Reported-by: brian m. carlson 
Reported-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 vcs-svn/repo_tree.h | 10 --
 1 file changed, 10 deletions(-)

diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
index 889c6a3c95..6c2f5f8a00 100644
--- a/vcs-svn/repo_tree.h
+++ b/vcs-svn/repo_tree.h
@@ -1,23 +1,13 @@
 #ifndef REPO_TREE_H_
 #define REPO_TREE_H_
 
-struct strbuf;
-
 #define REPO_MODE_DIR 004
 #define REPO_MODE_BLB 0100644
 #define REPO_MODE_EXE 0100755
 #define REPO_MODE_LNK 012
 
-uint32_t next_blob_mark(void);
 void repo_copy(uint32_t revision, const char *src, const char *dst);
-void repo_add(const char *path, uint32_t mode, uint32_t blob_mark);
 const char *repo_read_path(const char *path, uint32_t *mode_out);
 void repo_delete(const char *path);
-void repo_commit(uint32_t revision, const char *author,
-   const struct strbuf *log, const char *uuid, const char *url,
-   long unsigned timestamp);
-void repo_diff(uint32_t r1, uint32_t r2);
-void repo_init(void);
-void repo_reset(void);
 
 #endif
-- 
2.14.1.342.g6490525c54

[PATCH 0/4] vcs-svn: remove repo_tree library

[Jonathan Nieder]

Hi,

Stefan noticed that repo_init from vcs-svn/repo_tree.h conflicts with
repository.h[1].  Earlier brian m. carlson noticed the same thing[2].

Originally repo_tree.h was used to manage an in-memory representation
of the state of the svn tree being imported.  When that in-memory
representation was retired, we were lazy and left some utility
functions there.  Here is a patch series to finish cleaning up and
remove vcs-svn/repo_tree.h completely.

This is an alternative to bc/vcs-svn-cleanup from 'next'.  Those
patches weren't cc-ed to me and I missed them --- sorry about that.  I
can rebase on top of them if that is more convenient.

Thoughts of all kinds welcome, as always.

Thanks,
Jonathan

Jonathan Nieder (4):
  vcs-svn: remove prototypes for missing functions
  vcs-svn: remove custom mode constants
  vcs-svn: remove repo_delete wrapper function
  vcs-svn: move remaining repo_tree functions to fast_export.h

 Makefile  |  1 -
 vcs-svn/fast_export.c | 41 +
 vcs-svn/fast_export.h |  3 +++
 vcs-svn/repo_tree.c   | 48 
 vcs-svn/repo_tree.h   | 23 ---
 vcs-svn/svndump.c | 33 -
 6 files changed, 56 insertions(+), 93 deletions(-)
 delete mode 100644 vcs-svn/repo_tree.c
 delete mode 100644 vcs-svn/repo_tree.h

[1] https://public-inbox.org/git/20170822213501.5928-1-sbel...@google.com
[2] 
https://public-inbox.org/git/2017082122.26729-3-sand...@crustytoothpaste.net

Re: git send-email Cc with cruft not working as expected

[Stefan Beller]

+cc people from that thread

On Tue, Aug 22, 2017 at 4:30 PM, Jacob Keller  wrote:
> On Tue, Aug 22, 2017 at 4:18 PM, Stefan Beller  wrote:
>> On Tue, Aug 22, 2017 at 4:15 PM, Jacob Keller  wrote:
>>> Hi,
>>>
>>> I recently found an issue with git-send-email where it does not
>>> properly remove the cruft of an email address when sending using a Cc:
>>> line.
>>>
>>> The specific example is with a commit containing the following Cc line,
>>>
>>> Cc: sta...@vger.kernel.org # 4.10+
>>
>> Please see and discuss at
>> https://public-inbox.org/git/20170216174924.GB2625@localhost/
>
> I read that thread, and it addressed the problem of
>
> Cc:  # 4.10+
>
> but did not fix this case without the <> around the email address.
>
> Additionally I just discovered that the behavior here changes pretty
> drastically if you have Email::Validate installed, now it splits the
> address into multiple things:
>
> sta...@vger.kernel.org, #, 4.10+
>
> Thanks,
> Jake

Re: git send-email Cc with cruft not working as expected

[Jacob Keller]

On Tue, Aug 22, 2017 at 4:18 PM, Stefan Beller  wrote:
> On Tue, Aug 22, 2017 at 4:15 PM, Jacob Keller  wrote:
>> Hi,
>>
>> I recently found an issue with git-send-email where it does not
>> properly remove the cruft of an email address when sending using a Cc:
>> line.
>>
>> The specific example is with a commit containing the following Cc line,
>>
>> Cc: sta...@vger.kernel.org # 4.10+
>
> Please see and discuss at
> https://public-inbox.org/git/20170216174924.GB2625@localhost/

I read that thread, and it addressed the problem of

Cc:  # 4.10+

but did not fix this case without the <> around the email address.

Additionally I just discovered that the behavior here changes pretty
drastically if you have Email::Validate installed, now it splits the
address into multiple things:

sta...@vger.kernel.org, #, 4.10+

Thanks,
Jake

Re: git send-email Cc with cruft not working as expected

[Stefan Beller]

On Tue, Aug 22, 2017 at 4:15 PM, Jacob Keller  wrote:
> Hi,
>
> I recently found an issue with git-send-email where it does not
> properly remove the cruft of an email address when sending using a Cc:
> line.
>
> The specific example is with a commit containing the following Cc line,
>
> Cc: sta...@vger.kernel.org # 4.10+

Please see and discuss at
https://public-inbox.org/git/20170216174924.GB2625@localhost/

git send-email Cc with cruft not working as expected

[Jacob Keller]

Hi,

I recently found an issue with git-send-email where it does not
properly remove the cruft of an email address when sending using a Cc:
line.

The specific example is with a commit containing the following Cc line,

Cc: sta...@vger.kernel.org # 4.10+

which is the standard way Linux upstream expects the stable Ccs to be,
and I saw several examples of this in the past.

However, this gets converted into a cc of
"sta...@vger.kernel.org#4.10+" which isn't a valid address obviously.

This does work as expected if you remember to

Cc:  # 4.10+

I would have assumed that validate_address would kick in and let me
know that the address I'd given isn't valid, or something along those
lines.

I tried to come up with a test for this, but modifying t9001 seemed to
cause other failures and I couldn't detangle exactly how the tests fit
together.

Is this simply expected behavior and I need to remember to use <>
around the address?

Thanks,
Jake

[BUG] rebase -i with empty commits + exec

[Stefan Beller]

Currently I am working on a longer series, for which I decided
to keep track of progress in an empty commit. This empty commit
is in the middle of the series (to divide the commits into two sets,
the foundation that I consider stable and the later parts that are not
as stable for my development, they contain things that may be useful)

Then I invoked "git rebase -i  -x make" to see
in which shape the series is.

The editor opened proposing the following instruction sheet,
which in my opinion is buggy:

pick 1234 some commit
exec make
pick 2345 another commit
exec make
pick 3456 third commit
# pick 4567 empty commit
exec make
pick 5678  yet another commit
exec make

I think the lines of the empty commit and the following exec should
be swapped, because that exec should work on the third commit.
Maybe we'd want to see another commented exec:

pick 1234 some commit
exec make
pick 2345 another commit
exec make
pick 3456 third commit
exec make
# pick 4567 empty commit
# exec make <- unsure about this line
pick 5678  yet another commit
exec make

Thoughts?

Re: [PATCH] vcs-svn/repo_tree.h: remove repo_init declaration

[Jonathan Nieder]

Junio C Hamano wrote:
> Stefan Beller  writes:

>> The svn specific declaration of repo_init was not used since 723b7a2789
>> (vcs-svn: eliminate repo_tree structure, 2010-12-10).
>>
>> This was noticed when including repository.h via cache.h as that has the
>> same function with a different signature.
>>
>> Helped-by: Jonathan Nieder 
>> Signed-off-by: Stefan Beller 
>
> It looks to me that this is a reduced duplicate of what brian posted
> yesterday.  The first two patches in the 6-patch series that you
> commented on, I think, covers what this change wants to achieve and
> probably a lot more.  I've merged those two already to 'next' and
> was about to push the result out.

Thanks for the pointer. I believe you're referring to
https://public-inbox.org/git/2017082122.26729-2-sand...@crustytoothpaste.net/.

>From Stefan's verbal report I was about to send a patch just like
Brian's plus some more patches on top to get rid of the rest of repo_tree.h.
I can build on top of Brian's patch if that's more convenient.

Regards,
Jonathan

Re: [GSoC][PATCH 2/4] submodule--helper: introduce for_each_submodule_list()

[Junio C Hamano]

Prathamesh Chavan  writes:

> -static void init_submodule(const char *path, const char *prefix, int quiet)
> +static void for_each_submodule_list(const struct module_list list,
> + submodule_list_func_t fn, void *cb_data)

It may not be wrong per-se, but can't this just be for_each_submodule()?

Your "justification" may be that this makes it clear that you are
iterating over module_list and not other kind of group of
submodules, but I would say the design of the subsystem is broken if
some places use a list of submodules while some other places use an
array of submodules to represent a group of submodules.  Especially
when there is a dedicated type to hold a group of submodules,
i.e. struct module-list, that type should be used consistently
throughout the subsystem and API, no?

>  {
> + int i;
> + for (i = 0; i < list.nr; i++)
> + fn(list.entries[i], cb_data);
> +}

Also, did you really want to pass the structure by value?  At least
in C, it is more customary to pass these things by pointer, i.e.

for_each_submodule(struct module_list *list,
   for_each_submodule_fn fn,
   void *cb_data)
{
for (i = 0; i < list->nr; i++)
...

Otherwise you'd be making a copy on stack unnecessarily (ok, "const"
might hint a smart compiler to turn this inefficient code to pass it
by pointer, but I do not think it is a particulary good to rely on
such things).

Re: [GSoC][PATCH 1/4] submodule--helper: introduce get_submodule_displaypath()

[Junio C Hamano]

Prathamesh Chavan  writes:

> Introduce function get_submodule_displaypath() to replace the code
> occurring in submodule_init() for generating displaypath of the
> submodule with a call to it.

Looks like a quite straight-forward refactoring.

> +static char *get_submodule_displaypath(const char *path, const char *prefix)
> +{
> + const char *super_prefix = get_super_prefix();
> +
> + if (prefix && super_prefix) {
> + BUG("cannot have prefix '%s' and superprefix '%s'",
> + prefix, super_prefix);
> + } else if (prefix) {
> + struct strbuf sb = STRBUF_INIT;
> + char *displaypath = xstrdup(relative_path(path, prefix, ));
> + strbuf_release();
> + return displaypath;

Use of xstrdup() would waste one extra allocation and copy, no?  In
other words, wouldn't this do the same thing?

... {
struct strbuf sb = STRBUF_INIT;

relative_path(path, prefix, );
return strbuf_detach(, NULL);

> @@ -363,7 +375,6 @@ static void init_submodule(const char *path, const char 
> *prefix, int quiet)
>* Set active flag for the submodule being initialized
>*/
>   if (!is_submodule_active(the_repository, path)) {
> - strbuf_reset();
>   strbuf_addf(, "submodule.%s.active", sub->name);
>   git_config_set_gently(sb.buf, "true");
>   }

This is because sb will stay untouched with the use of the new
helper.  With the way this single strbuf is reused throughout this
function, I cannot help wondering if the prevailing pattern of
resetting and then using is a mistake.  If the strbuf is mostly used
as a scratchpad, wouldn't it make more sense to use it and then
clean up when you are done with it?

I.e. something along this line that shows only two uses...

 builtin/submodule--helper.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 0ff9dd0b85..84ded4b2e9 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -363,9 +363,9 @@ static void init_submodule(const char *path, const char 
*prefix, int quiet)
 * Set active flag for the submodule being initialized
 */
if (!is_submodule_active(the_repository, path)) {
-   strbuf_reset();
strbuf_addf(, "submodule.%s.active", sub->name);
git_config_set_gently(sb.buf, "true");
+   strbuf_reset();
}
 
/*
@@ -373,7 +373,6 @@ static void init_submodule(const char *path, const char 
*prefix, int quiet)
 * To look up the url in .git/config, we must not fall back to
 * .gitmodules, so look it up directly.
 */
-   strbuf_reset();
strbuf_addf(, "submodule.%s.url", sub->name);
if (git_config_get_string(sb.buf, )) {
if (!sub->url)
@@ -410,9 +409,9 @@ static void init_submodule(const char *path, const char 
*prefix, int quiet)
_("Submodule '%s' (%s) registered for path 
'%s'\n"),
sub->name, url, displaypath);
}
+   strbuf_reset();
 
/* Copy "update" setting when it is not set yet */
-   strbuf_reset();
strbuf_addf(, "submodule.%s.update", sub->name);
if (git_config_get_string(sb.buf, ) &&
sub->update_strategy.type != SM_UPDATE_UNSPECIFIED) {

Re: How to force a push to succeed?

[Jeffrey Walton]

 Commit seems to be the wrong command as Git appears to be trying to do
 something I don't want.

 How do I force the push to succeed?

 Thanks in advance.
>>>
>>> Checkout the --force[-with-lease] argument.
>
> Thanks again Stefan,
>
> From another testing machine, it looks like the changes have not been
> backed out. The previous operation un-did the ADX gear because it was
> an evolutionary dead-end.
>
> via$ git pull
> From https://github.com/noloader/cryptopp
>  + 66654dd...559fc3b master -> origin/master  (forced update)
> Already up-to-date.
> via:$ cat integer.cpp | grep ADX
> #if defined(CRYPTOPP_ADX_AVAILABLE)
> # define CRYPTOPP_INTEGER_ADX 1
> #if CRYPTOPP_INTEGER_SSE2 || CRYPTOPP_INTEGER_ADX
> #if CRYPTOPP_INTEGER_ADX
> extern int CRYPTOPP_FASTCALL ADX_Add(size_t N, word *C, const word *A,
> const word *B);
> #if CRYPTOPP_INTEGER_ADX
> if (HasADX())
> s_pAdd = _Add;
> #if CRYPTOPP_INTEGER_SSE2 || CRYPTOPP_INTEGER_ADX
>
> Above, there should be no references to ADX. Looking at the GitHub the
> changes have been applied.
>
> Any ideas?

You know, I look at how fucked up yet another simple workflow is, and
all I can do is wonder. It is absolutely amazing. Its like the project
goes out of its way to make simple tasks difficult.

Re: [PATCH] vcs-svn/repo_tree.h: remove repo_init declaration

[Stefan Beller]

On Tue, Aug 22, 2017 at 3:18 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>
>> The svn specific declaration of repo_init was not used since 723b7a2789
>> (vcs-svn: eliminate repo_tree structure, 2010-12-10).
>>
>> This was noticed when including repository.h via cache.h as that has the
>> same function with a different signature.
>>
>> Helped-by: Jonathan Nieder 
>> Signed-off-by: Stefan Beller 
>> ---
>
> It looks to me that this is a reduced duplicate of what brian posted
> yesterday.  The first two patches in the 6-patch series that you
> commented on, I think, covers what this change wants to achieve and
> probably a lot more.  I've merged those two already to 'next' and
> was about to push the result out.
>
> Thanks.
>

Ok, thanks. I did not remember reviewing those.
(I just wanted to fix my odd compile error here,
and currently I build a series on top of jt/packmigrate
so I shot off a quick one liner)

Sorry for the noise.

Re: [PATCH] vcs-svn/repo_tree.h: remove repo_init declaration

[Junio C Hamano]

Stefan Beller  writes:


> The svn specific declaration of repo_init was not used since 723b7a2789
> (vcs-svn: eliminate repo_tree structure, 2010-12-10).
>
> This was noticed when including repository.h via cache.h as that has the
> same function with a different signature.
>
> Helped-by: Jonathan Nieder 
> Signed-off-by: Stefan Beller 
> ---

It looks to me that this is a reduced duplicate of what brian posted
yesterday.  The first two patches in the 6-patch series that you
commented on, I think, covers what this change wants to achieve and
probably a lot more.  I've merged those two already to 'next' and
was about to push the result out.

Thanks.


>  vcs-svn/repo_tree.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
> index 889c6a3c95..8592beb59b 100644
> --- a/vcs-svn/repo_tree.h
> +++ b/vcs-svn/repo_tree.h
> @@ -17,7 +17,6 @@ void repo_commit(uint32_t revision, const char *author,
>   const struct strbuf *log, const char *uuid, const char *url,
>   long unsigned timestamp);
>  void repo_diff(uint32_t r1, uint32_t r2);
> -void repo_init(void);
>  void repo_reset(void);
>  
>  #endif

Re: How to force a push to succeed?

[Jeffrey Walton]

On Tue, Aug 22, 2017 at 6:03 PM, Jeffrey Walton  wrote:
> On Tue, Aug 22, 2017 at 5:57 PM, Stefan Beller  wrote:
>> On Tue, Aug 22, 2017 at 2:55 PM, Jeffrey Walton  wrote:
>>> I tested some changes that lead to a dead end. The changes need to be
>>> removed. The changes were added in 7 commits.
>>>
>>> I went back in time to the point before the changes:
>>>
>>> $ git reset --hard HEAD~7
>>> HEAD is now at 559fc3b Fix benchmark selection code (GH #464)
>>>
>>> When I attempted to push:
>>>
>>> $ git push
>>> Username for 'https://github.com': noloader
>>> To https://github.com/noloader/cryptopp.git
>>>  ! [rejected]master -> master (non-fast-forward)
>>>
>>> I tried to commit, but Git claims there's nothing to add:
>>>
>>> $ git commit
>>> On branch master
>>> Your branch is behind 'origin/master' by 7 commits, and can be
>>> fast-forwarded.
>>>
>>> Commit seems to be the wrong command as Git appears to be trying to do
>>> something I don't want.
>>>
>>> How do I force the push to succeed?
>>>
>>> Thanks in advance.
>>
>> Checkout the --force[-with-lease] argument.

Thanks again Stefan,

>From another testing machine, it looks like the changes have not been
backed out. The previous operation un-did the ADX gear because it was
an evolutionary dead-end.

via$ git pull
>From https://github.com/noloader/cryptopp
 + 66654dd...559fc3b master -> origin/master  (forced update)
Already up-to-date.
via:$ cat integer.cpp | grep ADX
#if defined(CRYPTOPP_ADX_AVAILABLE)
# define CRYPTOPP_INTEGER_ADX 1
#if CRYPTOPP_INTEGER_SSE2 || CRYPTOPP_INTEGER_ADX
#if CRYPTOPP_INTEGER_ADX
extern int CRYPTOPP_FASTCALL ADX_Add(size_t N, word *C, const word *A,
const word *B);
#if CRYPTOPP_INTEGER_ADX
if (HasADX())
s_pAdd = _Add;
#if CRYPTOPP_INTEGER_SSE2 || CRYPTOPP_INTEGER_ADX

Above, there should be no references to ADX. Looking at the GitHub the
changes have been applied.

Any ideas?

Jeff

Re: How to force a push to succeed?

[Jeffrey Walton]

On Tue, Aug 22, 2017 at 5:57 PM, Stefan Beller  wrote:
> On Tue, Aug 22, 2017 at 2:55 PM, Jeffrey Walton  wrote:
>> I tested some changes that lead to a dead end. The changes need to be
>> removed. The changes were added in 7 commits.
>>
>> I went back in time to the point before the changes:
>>
>> $ git reset --hard HEAD~7
>> HEAD is now at 559fc3b Fix benchmark selection code (GH #464)
>>
>> When I attempted to push:
>>
>> $ git push
>> Username for 'https://github.com': noloader
>> To https://github.com/noloader/cryptopp.git
>>  ! [rejected]master -> master (non-fast-forward)
>>
>> I tried to commit, but Git claims there's nothing to add:
>>
>> $ git commit
>> On branch master
>> Your branch is behind 'origin/master' by 7 commits, and can be
>> fast-forwarded.
>>
>> Commit seems to be the wrong command as Git appears to be trying to do
>> something I don't want.
>>
>> How do I force the push to succeed?
>>
>> Thanks in advance.
>
> Checkout the --force[-with-lease] argument.

Perfect, thanks.

Jeff

Re: How to force a push to succeed?

[Stefan Beller]

On Tue, Aug 22, 2017 at 2:55 PM, Jeffrey Walton  wrote:
> I tested some changes that lead to a dead end. The changes need to be
> removed. The changes were added in 7 commits.
>
> I went back in time to the point before the changes:
>
> $ git reset --hard HEAD~7
> HEAD is now at 559fc3b Fix benchmark selection code (GH #464)
>
> When I attempted to push:
>
> $ git push
> Username for 'https://github.com': noloader
> To https://github.com/noloader/cryptopp.git
>  ! [rejected]master -> master (non-fast-forward)
>
> I tried to commit, but Git claims there's nothing to add:
>
> $ git commit
> On branch master
> Your branch is behind 'origin/master' by 7 commits, and can be
> fast-forwarded.
>
> Commit seems to be the wrong command as Git appears to be trying to do
> something I don't want.
>
> How do I force the push to succeed?
>
> Thanks in advance.

Checkout the --force[-with-lease] argument.

https://git-scm.com/docs/git-push

How to force a push to succeed?

[Jeffrey Walton]

I tested some changes that lead to a dead end. The changes need to be
removed. The changes were added in 7 commits.

I went back in time to the point before the changes:

$ git reset --hard HEAD~7
HEAD is now at 559fc3b Fix benchmark selection code (GH #464)

When I attempted to push:

$ git push
Username for 'https://github.com': noloader
To https://github.com/noloader/cryptopp.git
 ! [rejected]master -> master (non-fast-forward)

I tried to commit, but Git claims there's nothing to add:

$ git commit
On branch master
Your branch is behind 'origin/master' by 7 commits, and can be
fast-forwarded.

Commit seems to be the wrong command as Git appears to be trying to do
something I don't want.

How do I force the push to succeed?

Thanks in advance.

[PATCH v2 6/6] rerere: allow approxidate in gc.rerereResolved/gc.rerereUnresolved

[Junio C Hamano]

These two configuration variables are described in the documentation
to take an expiry period expressed in the number of days:

gc.rerereResolved::
Records of conflicted merge you resolved earlier are
kept for this many days when 'git rerere gc' is run.
The default is 60 days.

gc.rerereUnresolved::
Records of conflicted merge you have not resolved are
kept for this many days when 'git rerere gc' is run.
The default is 15 days.

There is no strong reason not to allow a more general "approxidate"
expiry specification, e.g. "5.days.ago", or "never".

Rename the config_get_expiry() helper introduced in the previous
step to git_config_get_expiry_in_days() and move it to a more
generic place, config.c, and use date.c::parse_expiry_date() to do
so.  Give it an ability to allow the caller to tell among three
cases (i.e. there is no "gc.rerereResolved" config, there is and it
is correctly parsed into the *expiry variable, and there was an
error in parsing the given value).  The current caller can work
correctly without using the return value, though.

In the future, we may find other variables that only allow an
integer that specifies "this many days" or other unit of time, and
when it happens we may need to drop "_days" suffix from the name of
the function and instead pass the "scale" value as another parameter.

But this will do for now.

Signed-off-by: Junio C Hamano 
---
 Documentation/config.txt |  2 ++
 config.c | 22 ++
 config.h |  3 +++
 rerere.c | 14 ++
 t/t4200-rerere.sh|  2 ++
 5 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d5c9c4cab6..ac95f5f954 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1553,11 +1553,13 @@ gc..reflogExpireUnreachable::
 gc.rerereResolved::
Records of conflicted merge you resolved earlier are
kept for this many days when 'git rerere gc' is run.
+   You can also use more human-readable "1.month.ago", etc.
The default is 60 days.  See linkgit:git-rerere[1].
 
 gc.rerereUnresolved::
Records of conflicted merge you have not resolved are
kept for this many days when 'git rerere gc' is run.
+   You can also use more human-readable "1.month.ago", etc.
The default is 15 days.  See linkgit:git-rerere[1].
 
 gitcvs.commitMsgAnnotation::
diff --git a/config.c b/config.c
index 231f9a750b..ffca15f594 100644
--- a/config.c
+++ b/config.c
@@ -2066,6 +2066,28 @@ int git_config_get_expiry(const char *key, const char 
**output)
return ret;
 }
 
+int git_config_get_expiry_in_days(const char *key, timestamp_t *expiry, 
timestamp_t now)
+{
+   char *expiry_string;
+   intmax_t days;
+   timestamp_t when;
+
+   if (git_config_get_string(key, _string))
+   return 1; /* no such thing */
+
+   if (git_parse_signed(expiry_string, , 
maximum_signed_value_of_type(int))) {
+   const int scale = 86400;
+   *expiry = now - days * scale;
+   return 0;
+   }
+
+   if (!parse_expiry_date(expiry_string, )) {
+   *expiry = when;
+   return 0;
+   }
+   return -1; /* thing exists but cannot be parsed */
+}
+
 int git_config_get_untracked_cache(void)
 {
int val = -1;
diff --git a/config.h b/config.h
index 0352da117b..34ddd3eb8d 100644
--- a/config.h
+++ b/config.h
@@ -205,6 +205,9 @@ extern int git_config_get_max_percent_split_change(void);
 /* This dies if the configured or default date is in the future */
 extern int git_config_get_expiry(const char *key, const char **output);
 
+/* parse either "this many days" integer, or "5.days.ago" approxidate */
+extern int git_config_get_expiry_in_days(const char *key, timestamp_t *, 
timestamp_t now);
+
 struct key_value_info {
const char *filename;
int linenr;
diff --git a/rerere.c b/rerere.c
index f0b4bce881..d77235645e 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1176,16 +1176,6 @@ static void prune_one(struct rerere_id *id,
unlink_rr_item(id);
 }
 
-static void config_get_expiry(const char *key, timestamp_t *cutoff, 
timestamp_t now)
-{
-   int days;
-
-   if (!git_config_get_int(key, )) {
-   const int scale = 86400;
-   *cutoff = now - days * scale;
-   }
-}
-
 void rerere_gc(struct string_list *rr)
 {
struct string_list to_remove = STRING_LIST_INIT_DUP;
@@ -1199,8 +1189,8 @@ void rerere_gc(struct string_list *rr)
if (setup_rerere(rr, 0) < 0)
return;
 
-   config_get_expiry("gc.rerereresolved", _resolve, now);
-   config_get_expiry("gc.rerereunresolved", _noresolve, now);
+   git_config_get_expiry_in_days("gc.rerereresolved", _resolve, 
now);
+   git_config_get_expiry_in_days("gc.rerereunresolved", 

[PATCH v2 5/6] rerere: represent time duration in timestamp_t internally

[Junio C Hamano]

The two configuration variables, gc.rerereResolved and
gc.rerereUnresolved, are measured in days and are passed as such
into the prune_one() helper function, which worked in time_t to see
if an entry in the rerere database is past its expiry.

Instead, have the caller turn the number of days into the expiry
timestamp.  Further, use timestamp_t instead of time_t.  This will
make it possible to extend the way the configuration variable is
spelled by using date.c::parse_expiry_date() that gives the expiry
timestamp in timestamp_t.

Signed-off-by: Junio C Hamano 
---
 rerere.c | 36 +++-
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/rerere.c b/rerere.c
index 70634d456c..f0b4bce881 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1133,14 +1133,14 @@ int rerere_forget(struct pathspec *pathspec)
  * Garbage collection support
  */
 
-static time_t rerere_created_at(struct rerere_id *id)
+static timestamp_t rerere_created_at(struct rerere_id *id)
 {
struct stat st;
 
return stat(rerere_path(id, "preimage"), ) ? (time_t) 0 : 
st.st_mtime;
 }
 
-static time_t rerere_last_used_at(struct rerere_id *id)
+static timestamp_t rerere_last_used_at(struct rerere_id *id)
 {
struct stat st;
 
@@ -1157,11 +1157,11 @@ static void unlink_rr_item(struct rerere_id *id)
id->collection->status[id->variant] = 0;
 }
 
-static void prune_one(struct rerere_id *id, time_t now,
- int cutoff_resolve, int cutoff_noresolve)
+static void prune_one(struct rerere_id *id,
+ timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)
 {
-   time_t then;
-   int cutoff;
+   timestamp_t then;
+   timestamp_t cutoff;
 
then = rerere_last_used_at(id);
if (then)
@@ -1172,25 +1172,35 @@ static void prune_one(struct rerere_id *id, time_t now,
return;
cutoff = cutoff_noresolve;
}
-   if (then < now - cutoff * 86400)
+   if (then < cutoff)
unlink_rr_item(id);
 }
 
+static void config_get_expiry(const char *key, timestamp_t *cutoff, 
timestamp_t now)
+{
+   int days;
+
+   if (!git_config_get_int(key, )) {
+   const int scale = 86400;
+   *cutoff = now - days * scale;
+   }
+}
+
 void rerere_gc(struct string_list *rr)
 {
struct string_list to_remove = STRING_LIST_INIT_DUP;
DIR *dir;
struct dirent *e;
int i;
-   time_t now = time(NULL);
-   int cutoff_noresolve = 15;
-   int cutoff_resolve = 60;
+   timestamp_t now = time(NULL);
+   timestamp_t cutoff_noresolve = now - 15 * 86400;
+   timestamp_t cutoff_resolve = now - 60 * 86400;
 
if (setup_rerere(rr, 0) < 0)
return;
 
-   git_config_get_int("gc.rerereresolved", _resolve);
-   git_config_get_int("gc.rerereunresolved", _noresolve);
+   config_get_expiry("gc.rerereresolved", _resolve, now);
+   config_get_expiry("gc.rerereunresolved", _noresolve, now);
git_config(git_default_config, NULL);
dir = opendir(git_path("rr-cache"));
if (!dir)
@@ -1211,7 +1221,7 @@ void rerere_gc(struct string_list *rr)
for (id.variant = 0, id.collection = rr_dir;
 id.variant < id.collection->status_nr;
 id.variant++) {
-   prune_one(, now, cutoff_resolve, cutoff_noresolve);
+   prune_one(, cutoff_resolve, cutoff_noresolve);
if (id.collection->status[id.variant])
now_empty = 0;
}
-- 
2.14.1-427-g5711bb0564

[PATCH v2 3/6] t4200: gather "rerere gc" together

[Junio C Hamano]

Move the "rerere gc with custom expiry" test up, so that it is close
to the more "rerere gc" tests.

Signed-off-by: Junio C Hamano 
---
 t/t4200-rerere.sh | 54 +++---
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 1e23031cdb..b007b67e9a 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -239,6 +239,33 @@ test_expect_success 'old records rest in peace' '
! test -f $rr2/preimage
 '
 
+test_expect_success 'rerere gc with custom expiry' '
+   rm -fr .git/rr-cache &&
+   rr=.git/rr-cache/$_z40 &&
+   mkdir -p "$rr" &&
+   >"$rr/preimage" &&
+   >"$rr/postimage" &&
+
+   two_days_ago=$((-2*86400)) &&
+   test-chmtime =$two_days_ago "$rr/preimage" &&
+   test-chmtime =$two_days_ago "$rr/postimage" &&
+
+   find .git/rr-cache -type f | sort >original &&
+
+   git -c gc.rerereresolved=5 -c gc.rerereunresolved=5 rerere gc &&
+   find .git/rr-cache -type f | sort >actual &&
+   test_cmp original actual &&
+
+   git -c gc.rerereresolved=5 -c gc.rerereunresolved=0 rerere gc &&
+   find .git/rr-cache -type f | sort >actual &&
+   test_cmp original actual &&
+
+   git -c gc.rerereresolved=0 -c gc.rerereunresolved=0 rerere gc &&
+   find .git/rr-cache -type f | sort >actual &&
+   >expect &&
+   test_cmp expect actual
+'
+
 test_expect_success 'setup: file2 added differently in two branches' '
git reset --hard &&
 
@@ -419,33 +446,6 @@ count_pre_post () {
test_line_count = "$2" actual
 }
 
-test_expect_success 'rerere gc' '
-   rm -fr .git/rr-cache &&
-   rr=.git/rr-cache/$_z40 &&
-   mkdir -p "$rr" &&
-   >"$rr/preimage" &&
-   >"$rr/postimage" &&
-
-   two_days_ago=$((-2*86400)) &&
-   test-chmtime =$two_days_ago "$rr/preimage" &&
-   test-chmtime =$two_days_ago "$rr/postimage" &&
-
-   find .git/rr-cache -type f | sort >original &&
-
-   git -c gc.rerereresolved=5 -c gc.rerereunresolved=5 rerere gc &&
-   find .git/rr-cache -type f | sort >actual &&
-   test_cmp original actual &&
-
-   git -c gc.rerereresolved=5 -c gc.rerereunresolved=0 rerere gc &&
-   find .git/rr-cache -type f | sort >actual &&
-   test_cmp original actual &&
-
-   git -c gc.rerereresolved=0 -c gc.rerereunresolved=0 rerere gc &&
-   find .git/rr-cache -type f | sort >actual &&
-   >expect &&
-   test_cmp expect actual
-'
-
 merge_conflict_resolve () {
git reset --hard &&
test_must_fail git merge six.1 &&
-- 
2.14.1-427-g5711bb0564

[PATCH v2 4/6] t4200: parameterize "rerere gc" custom expiry test

[Junio C Hamano]

The test creates a rerere database entry that is two days old, and
tries to expire with three different custom expiry configuration
(keep ones less than 5 days old, keep ones used less than 5 days
ago, and expire everything right now).

We'll be introducing a different way to spell the same "5 days" and
"right now" parameter in a later step; parameterize the test to make
it easier to test the new spelling when it happens.

Signed-off-by: Junio C Hamano 
---
 t/t4200-rerere.sh | 58 +++
 1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index b007b67e9a..8d437534f2 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -239,32 +239,40 @@ test_expect_success 'old records rest in peace' '
! test -f $rr2/preimage
 '
 
-test_expect_success 'rerere gc with custom expiry' '
-   rm -fr .git/rr-cache &&
-   rr=.git/rr-cache/$_z40 &&
-   mkdir -p "$rr" &&
-   >"$rr/preimage" &&
-   >"$rr/postimage" &&
-
-   two_days_ago=$((-2*86400)) &&
-   test-chmtime =$two_days_ago "$rr/preimage" &&
-   test-chmtime =$two_days_ago "$rr/postimage" &&
-
-   find .git/rr-cache -type f | sort >original &&
-
-   git -c gc.rerereresolved=5 -c gc.rerereunresolved=5 rerere gc &&
-   find .git/rr-cache -type f | sort >actual &&
-   test_cmp original actual &&
-
-   git -c gc.rerereresolved=5 -c gc.rerereunresolved=0 rerere gc &&
-   find .git/rr-cache -type f | sort >actual &&
-   test_cmp original actual &&
+rerere_gc_custom_expiry_test () {
+   five_days="$1" right_now="$2"
+   test_expect_success "rerere gc with custom expiry ($five_days, 
$right_now)" '
+   rm -fr .git/rr-cache &&
+   rr=.git/rr-cache/$_z40 &&
+   mkdir -p "$rr" &&
+   >"$rr/preimage" &&
+   >"$rr/postimage" &&
+
+   two_days_ago=$((-2*86400)) &&
+   test-chmtime =$two_days_ago "$rr/preimage" &&
+   test-chmtime =$two_days_ago "$rr/postimage" &&
+
+   find .git/rr-cache -type f | sort >original &&
+
+   git -c "gc.rerereresolved=$five_days" \
+   -c "gc.rerereunresolved=$five_days" rerere gc &&
+   find .git/rr-cache -type f | sort >actual &&
+   test_cmp original actual &&
+
+   git -c "gc.rerereresolved=$five_days" \
+   -c "gc.rerereunresolved=$right_now" rerere gc &&
+   find .git/rr-cache -type f | sort >actual &&
+   test_cmp original actual &&
+
+   git -c "gc.rerereresolved=$right_now" \
+   -c "gc.rerereunresolved=$right_now" rerere gc &&
+   find .git/rr-cache -type f | sort >actual &&
+   >expect &&
+   test_cmp expect actual
+   '
+}
 
-   git -c gc.rerereresolved=0 -c gc.rerereunresolved=0 rerere gc &&
-   find .git/rr-cache -type f | sort >actual &&
-   >expect &&
-   test_cmp expect actual
-'
+rerere_gc_custom_expiry_test 5 0
 
 test_expect_success 'setup: file2 added differently in two branches' '
git reset --hard &&
-- 
2.14.1-427-g5711bb0564

[PATCH v2 2/6] t4200: make "rerere gc" test more robust

[Junio C Hamano]

The test blindly trusted that there may be _some_ entries left in
the rerere database, and used them by updating their timestamps to
see if the gc threshold variables are honoured correctly.  This
won't work if there is no entries in the database when the test
begins.

Instead, clear the rerere database, and populate it with a few known
entries (which are bogus, but for the purpose of testing "garbage
collection", it does not matter---we want to make sure we collect
old cruft, even if the files are corrupt rerere database entries),
and use them for the expiry test.

Signed-off-by: Junio C Hamano 
---
 t/t4200-rerere.sh | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 8f5f268baf..1e23031cdb 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -420,19 +420,28 @@ count_pre_post () {
 }
 
 test_expect_success 'rerere gc' '
-   find .git/rr-cache -type f >original &&
-   xargs test-chmtime -172800 "$rr/preimage" &&
+   >"$rr/postimage" &&
+
+   two_days_ago=$((-2*86400)) &&
+   test-chmtime =$two_days_ago "$rr/preimage" &&
+   test-chmtime =$two_days_ago "$rr/postimage" &&
+
+   find .git/rr-cache -type f | sort >original &&
 
git -c gc.rerereresolved=5 -c gc.rerereunresolved=5 rerere gc &&
-   find .git/rr-cache -type f >actual &&
+   find .git/rr-cache -type f | sort >actual &&
test_cmp original actual &&
 
git -c gc.rerereresolved=5 -c gc.rerereunresolved=0 rerere gc &&
-   find .git/rr-cache -type f >actual &&
+   find .git/rr-cache -type f | sort >actual &&
test_cmp original actual &&
 
git -c gc.rerereresolved=0 -c gc.rerereunresolved=0 rerere gc &&
-   find .git/rr-cache -type f >actual &&
+   find .git/rr-cache -type f | sort >actual &&
>expect &&
test_cmp expect actual
 '
-- 
2.14.1-427-g5711bb0564

[PATCH v2 0/6] accept non-integer for "this many days" expiry specification

[Junio C Hamano]

About a month ago, we wondered why

[gc]
rerereResolved = 5.days

does not work and if we want to do something better.  

This is an update to my first attempt.  It turns out that I needed
more patches to clean up the tests before I can write new tests for
this feature.

The implementation of the feature itself has not changed much,
except that it now lives in config.c for easier reuse by other
codepaths in the future.

Junio C Hamano (6):
  t4200: give us a clean slate after "rerere gc" tests
  t4200: make "rerere gc" test more robust
  t4200: gather "rerere gc" together
  t4200: parameterize "rerere gc" custom expiry test
  rerere: represent time duration in timestamp_t internally
  rerere: allow approxidate in gc.rerereResolved/gc.rerereUnresolved

 Documentation/config.txt |  2 ++
 config.c | 22 +++
 config.h |  3 +++
 rerere.c | 26 +++---
 t/t4200-rerere.sh| 57 +---
 5 files changed, 79 insertions(+), 31 deletions(-)


The interdiff between the original one and this version looks like
this.

 config.c  | 26 +++--
 config.h  |  6 +++---
 rerere.c  | 24 ++-
 t/t4200-rerere.sh | 57 +--
 4 files changed, 68 insertions(+), 45 deletions(-)

diff --git a/config.c b/config.c
index ac9071c5cf..ffca15f594 100644
--- a/config.c
+++ b/config.c
@@ -769,7 +769,7 @@ static int parse_unit_factor(const char *end, uintmax_t 
*val)
return 0;
 }
 
-int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)
+static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)
 {
if (value && *value) {
char *end;
@@ -799,7 +799,7 @@ int git_parse_signed(const char *value, intmax_t *ret, 
intmax_t max)
return 0;
 }
 
-int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max)
+static int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max)
 {
if (value && *value) {
char *end;
@@ -2066,6 +2066,28 @@ int git_config_get_expiry(const char *key, const char 
**output)
return ret;
 }
 
+int git_config_get_expiry_in_days(const char *key, timestamp_t *expiry, 
timestamp_t now)
+{
+   char *expiry_string;
+   intmax_t days;
+   timestamp_t when;
+
+   if (git_config_get_string(key, _string))
+   return 1; /* no such thing */
+
+   if (git_parse_signed(expiry_string, , 
maximum_signed_value_of_type(int))) {
+   const int scale = 86400;
+   *expiry = now - days * scale;
+   return 0;
+   }
+
+   if (!parse_expiry_date(expiry_string, )) {
+   *expiry = when;
+   return 0;
+   }
+   return -1; /* thing exists but cannot be parsed */
+}
+
 int git_config_get_untracked_cache(void)
 {
int val = -1;
diff --git a/config.h b/config.h
index 039a9295de..34ddd3eb8d 100644
--- a/config.h
+++ b/config.h
@@ -205,6 +205,9 @@ extern int git_config_get_max_percent_split_change(void);
 /* This dies if the configured or default date is in the future */
 extern int git_config_get_expiry(const char *key, const char **output);
 
+/* parse either "this many days" integer, or "5.days.ago" approxidate */
+extern int git_config_get_expiry_in_days(const char *key, timestamp_t *, 
timestamp_t now);
+
 struct key_value_info {
const char *filename;
int linenr;
@@ -215,7 +218,4 @@ struct key_value_info {
 extern NORETURN void git_die_config(const char *key, const char *err, ...) 
__attribute__((format(printf, 2, 3)));
 extern NORETURN void git_die_config_linenr(const char *key, const char 
*filename, int linenr);
 
-int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max);
-int git_parse_signed(const char *value, intmax_t *ret, intmax_t max);
-
 #endif /* CONFIG_H */
diff --git a/rerere.c b/rerere.c
index 8bbdfe8569..d77235645e 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1176,26 +1176,6 @@ static void prune_one(struct rerere_id *id,
unlink_rr_item(id);
 }
 
-static void config_get_expiry(const char *key, timestamp_t *cutoff, 
timestamp_t now)
-{
-   char *expiry_string;
-   intmax_t days;
-   timestamp_t when;
-
-   if (git_config_get_string(key, _string))
-   return;
-
-   if (git_parse_signed(expiry_string, , 
maximum_signed_value_of_type(int))) {
-   const int scale = 86400;
-   *cutoff = now - days * scale;
-   return;
-   }
-
-   if (!parse_expiry_date(expiry_string, )) {
-   *cutoff = when;
-   }
-}
-
 void rerere_gc(struct string_list *rr)
 {
struct string_list to_remove = STRING_LIST_INIT_DUP;
@@ -1209,8 +1189,8 @@ void rerere_gc(struct string_list *rr)
if (setup_rerere(rr, 0) < 0)
return;
 
-  

[PATCH v2 1/6] t4200: give us a clean slate after "rerere gc" tests

[Junio C Hamano]

The "multiple identical conflicts" test counts the number of entries
in the rerere database after trying a handful of mergy operations
and recording their resolutions, but without initializing the rerere
database to a known state, allowing the state left by previous tests
to trigger a false failure.  Make it robust by cleaning the database
before it starts.

Signed-off-by: Junio C Hamano 
---
 t/t4200-rerere.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 1a080e7823..8f5f268baf 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -446,6 +446,8 @@ merge_conflict_resolve () {
 }
 
 test_expect_success 'multiple identical conflicts' '
+   rm -fr .git/rr-cache &&
+   mkdir .git/rr-cache &&
git reset --hard &&
 
test_seq 1 6 >early &&
-- 
2.14.1-427-g5711bb0564

[PATCH] vcs-svn/repo_tree.h: remove repo_init declaration

[Stefan Beller]

The svn specific declaration of repo_init was not used since 723b7a2789
(vcs-svn: eliminate repo_tree structure, 2010-12-10).

This was noticed when including repository.h via cache.h as that has the
same function with a different signature.

Helped-by: Jonathan Nieder 
Signed-off-by: Stefan Beller 
---
 vcs-svn/repo_tree.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
index 889c6a3c95..8592beb59b 100644
--- a/vcs-svn/repo_tree.h
+++ b/vcs-svn/repo_tree.h
@@ -17,7 +17,6 @@ void repo_commit(uint32_t revision, const char *author,
const struct strbuf *log, const char *uuid, const char *url,
long unsigned timestamp);
 void repo_diff(uint32_t r1, uint32_t r2);
-void repo_init(void);
 void repo_reset(void);
 
 #endif
-- 
2.14.0.rc0.3.g6c2e499285

Re: [git-for-windows] Re: Revision resolution for remote-helpers?

[Johannes Schindelin]

Hi,

On Fri, 18 Aug 2017, Jonathan Nieder wrote:

> Mike Hommey wrote[1]:
> > On Fri, Aug 18, 2017 at 03:06:37PM -0700, Jonathan Nieder wrote:
> >> Mike Hommey wrote:
> 
> >>> The reason for the :: prefix is that it matches the ::
> >>> prefix used for remote helpers.
> >>>
> >>> Now, there are a few caveats:
> [...]
> >>> - msys likes to completely fuck up command lines when they contain ::.
> >>>   For remote helpers, the alternative that works is
> >>>   :///etc.
> >>
> >> Hm --- is there a bug already open about this (e.g. in the Git for
> >> Windows project or in msys) where I can read more?
> >
> > It's entirely an msys problem. Msys has weird rules to translate between
> > unix paths and windows paths on the command line, and botches everything
> > as a consequence. That's by "design".
> >
> > http://www.mingw.org/wiki/Posix_path_conversion
> >
> > (Particularly, see the last two entries)
> >
> > That only happens when calling native Windows programs from a msys
> > shell.
> 
> Cc-ing the Git for Windows mailing list as an FYI.
> 
> I have faint memories that some developers on that project have had to
> delve deep into Msys path modification rules.  It's possible they can
> give us advice (e.g. about :: having been a bad choice of
> syntax in the first place :)).

I think it is safe to assume that :: is not part of any Unix-y path. That
is why the MSYS2 runtime does not try to play games with it by converting
it to a Windows path.

(And indeed, I just tested this, an argument of the form
"a::file://b/x/y/z" is not converted to a "Windows path")

Ciao,
Dscho

Advice needed for basic setup for home user

[Harry Putnam]

Setup: Running openindian/hipster updated as of Aug 20 '17
Hardware: HP xw8600 - 2x Xeon  CPU X5450 @ 3.00GHz - 32 GB ram

I used cvs for several yrs before moving to git about yr ago.

In both cases I've barely scratched the surface with my usage.

I run 5-10 vbox vms' on this host with various OS's involved.
With each host, I've kept a local repo of some key OS rc files.
and a couple of hundred home made scripts. 

They all follow the same pattern of setup, but over time each repo
becomes different from its cousins. 

I've never taken the step of centralizing the assorted local git repos
into a central repo that keeps a branch or directory or whatever it
would be called of each local repo.

So that all the local repos would become a checked out module from the
central git repo.

Or at any rate, something along that line... not even sure how I would
set that up with git, but would like some overall advice about how to
do that. A step thru or an outline would be very useful.

Re: [PATCH] Add 'raw' blob_plain link in history overview

[Junio C Hamano]

Job Snijders  writes:

> For people that work with very large plain text files it may be easier
> if one can bypass viewing the htmlized blob and instead click directly
> to the raw file (rather then click through 'blob' and then to 'raw').
>
> Reviewed-by: Giuseppe Bilotta 
> Signed-off-by: Job Snijders 
>
> ---

This is much more readable ;-)  Will replace.

Thanks.

>  gitweb/gitweb.perl | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 9208f42ed..959f04b49 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -5967,6 +5967,9 @@ sub git_history_body {
> $cgi->a({-href => href(action=>"commitdiff", 
> hash=>$commit)}, "commitdiff");
>  
>   if ($ftype eq 'blob') {
> + print " | " .
> +   $cgi->a({-href => href(action=>"blob_plain", 
> hash_base=>$commit, file_name=>$file_name)}, "raw");
> +
>   my $blob_current = $file_hash;
>   my $blob_parent  = git_get_hash_by_path($commit, 
> $file_name);
>   if (defined $blob_current && defined $blob_parent &&

Re: Best way to check whether working tree matches a commit's tree

[Sebastian Schuberth]

On Tue, Aug 22, 2017 at 9:34 PM, Junio C Hamano  wrote:

>> While this works, it feels sub-optimal. Is there a better / smarter way?
>
> I do not think so; you want three things to match and you have a way
> to compare two things at a time.

Right. I was just thinking if there's a lesser known command like "git
diff --no-index", but instead of taking two paths, take just one path
and a commit.

> By the way, I think your second check should compare
>
> rev-parse HEAD^{tree} $that_commit^{tree}
>
> as you are checking if the tree exactly matches.

In fact, I was considering to use "git diff HEAD $that_commit" as I
don't really care whether the SHA1s are equal, but just about the file
contents / tree.

-- 
Sebastian Schuberth

Re: [PATCH] Add 'raw' blob_plain link in history overview

[Job Snijders]

For people that work with very large plain text files it may be easier
if one can bypass viewing the htmlized blob and instead click directly
to the raw file (rather then click through 'blob' and then to 'raw').

Reviewed-by: Giuseppe Bilotta 
Signed-off-by: Job Snijders 

---
 gitweb/gitweb.perl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 9208f42ed..959f04b49 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5967,6 +5967,9 @@ sub git_history_body {
  $cgi->a({-href => href(action=>"commitdiff", 
hash=>$commit)}, "commitdiff");
 
if ($ftype eq 'blob') {
+   print " | " .
+ $cgi->a({-href => href(action=>"blob_plain", 
hash_base=>$commit, file_name=>$file_name)}, "raw");
+
my $blob_current = $file_hash;
my $blob_parent  = git_get_hash_by_path($commit, 
$file_name);
if (defined $blob_current && defined $blob_parent &&

Re: [PATCH] Add 'raw' blob_plain link in history overview

[Junio C Hamano]

Job Snijders  writes:

> On Tue, Aug 22, 2017 at 12:22:43PM -0700, Junio C Hamano wrote:
>> Job Snijders  writes:
>> > Add 'raw' blob_plain link in history overview
>> >
>> > Reviewed-by: Giuseppe Bilotta 
>> > Signed-off-by: Job Snijders 
>> >
>> > ---
>> 
>> Thanks; I somehow thought that your earlier one not just said what
>> it does (twice---that is not needed) but why this change is useful,
>> but that is lost in the patch description?
>
> We often work with very large plain text files...

You do not have to explain it to _me_ ;-) I wanted you to explain it
to our history, instead of me manually tweaking your proposed log
message in your patch with what you sent over e-mail in a follow up
explanation like this one.

> ...
> immediately click to the 'raw' version, saving time and improving
> workflow.

Most of the above, or at least a condensed version of it, should be
in the proposed log message.  That is all I was saying.

Thanks.

Re: Cannot checkout after setting the eol attribute

[Junio C Hamano]

Torsten Bögershausen  writes:

> The following would solve your problem:
>git init
>echo $'dos\r' > dos
>git add dos
>git commit -m "dos newlines"
>echo "dos -crlf" > .gitattributes
>git add .gitattributes
>git commit -m "add attributes"
>echo "dos eol=crlf" > .gitattributes

>git read-tree --empty   # Clean index, force re-scan of working directory
>git add dos

This is not advisable as a general suggestion, as in a real life (as
opposed to a toy example) there will be paths other than this 'dos'
thing in the working tree.  Perhaps replace "git read-tree --empty"
with "git rm -f --cached dos" or something like that that limits its
damage would prevent mistakes better.

>git add .gitattributes
>git commit -m "set eol attribute instead"
>git ls-files --eol

Re: [PATCH] Add 'raw' blob_plain link in history overview

[Giuseppe Bilotta]

On Tue, Aug 22, 2017 at 9:35 PM, Job Snijders  wrote:

> We often work with very large plain text files in our repositories and
> found it friendlier to the users if we can click directly to the raw
> version of such files.

It might be worth it to add this information to the commit message.

-- 
Giuseppe "Oblomov" Bilotta

What's cooking in git.git (Aug 2017, #05; Tue, 22)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

The second batch of topics are in.  This cycle is going a bit slower
than the previous one (as of mid-week #3 of this cycle, we have
about 200 patches on 'master' since v2.14, compared to about 300
patches in the cycle towards v2.14 at a similar point in the cycle),
but hopefully we can catch up eventually.  

I am planning to be offline most of the next week, by the way.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* ab/ref-filter-no-contains (2017-08-07) 1 commit
  (merged to 'next' on 2017-08-18 at 7ec9d3d3a2)
 + tests: don't give unportable ">" to "test" built-in, use -gt

 A test fix.


* bw/clone-recursive-quiet (2017-08-04) 1 commit
  (merged to 'next' on 2017-08-14 at fbd4473ce4)
 + clone: teach recursive clones to respect -q

 "git clone --recurse-submodules --quiet" did not pass the quiet
 option down to submodules.


* bw/grep-recurse-submodules (2017-08-02) 10 commits
  (merged to 'next' on 2017-08-14 at dcfcfc94af)
 + grep: recurse in-process using 'struct repository'
 + submodule: merge repo_read_gitmodules and gitmodules_config
 + submodule: check for unmerged .gitmodules outside of config parsing
 + submodule: check for unstaged .gitmodules outside of config parsing
 + submodule: remove fetch.recursesubmodules from submodule-config parsing
 + submodule: remove submodule.fetchjobs from submodule-config parsing
 + config: add config_from_gitmodules
 + cache.h: add GITMODULES_FILE macro
 + repository: have the_repository use the_index
 + repo_read_index: don't discard the index
 (this branch is used by bw/submodule-config-cleanup.)

 "git grep --recurse-submodules" has been reworked to give a more
 consistent output across submodule boundary (and do its thing
 without having to fork a separate process).


* bw/push-options-recursively-to-submodules (2017-07-20) 1 commit
  (merged to 'next' on 2017-08-14 at 421dc09fd0)
 + submodule--helper: teach push-check to handle HEAD

 "git push --recurse-submodules $there HEAD:$target" was not
 propagated down to the submodules, but now it is.


* jc/perl-git-comment-typofix (2017-08-07) 1 commit
  (merged to 'next' on 2017-08-18 at b2ad043e6a)
 + perl/Git.pm: typofix in a comment

 A comment fix.


* jk/drop-sha1-entry-pos (2017-08-09) 1 commit
  (merged to 'next' on 2017-08-18 at 3a4d9bcf12)
 + sha1_file: drop experimental GIT_USE_LOOKUP search
 (this branch is used by jt/packmigrate.)

 Code clean-up.


* jk/hashcmp-memcmp (2017-08-09) 1 commit
  (merged to 'next' on 2017-08-18 at 27c4aa5520)
 + hashcmp: use memcmp instead of open-coded loop

 Code clean-up.


* ma/parse-maybe-bool (2017-08-07) 6 commits
  (merged to 'next' on 2017-08-18 at ba22bb836c)
 + parse_decoration_style: drop unused argument `var`
 + treewide: deprecate git_config_maybe_bool, use git_parse_maybe_bool
 + config: make git_{config,parse}_maybe_bool equivalent
 + config: introduce git_parse_maybe_bool_text
 + t5334: document that git push --signed=1 does not work
 + Doc/git-{push,send-pack}: correct --sign= to --signed=

 Code clean-up.


* mf/no-dashed-subcommands (2017-08-07) 1 commit
  (merged to 'next' on 2017-08-18 at 05365af2ff)
 + scripts: use "git foo" not "git-foo"

 Code clean-up.


* mh/packed-ref-store (2017-08-17) 32 commits
  (merged to 'next' on 2017-08-18 at 14c58936e1)
 + files-backend: cheapen refname_available check when locking refs
  (merged to 'next' on 2017-08-14 at 987b76d302)
 + packed_ref_store: handle a packed-refs file that is a symlink
 + read_packed_refs(): die if `packed-refs` contains bogus data
 + t3210: add some tests of bogus packed-refs file contents
 + repack_without_refs(): don't lock or unlock the packed refs
 + commit_packed_refs(): remove call to `packed_refs_unlock()`
 + clear_packed_ref_cache(): don't protest if the lock is held
 + packed_refs_unlock(), packed_refs_is_locked(): new functions
 + packed_refs_lock(): report errors via a `struct strbuf *err`
 + packed_refs_lock(): function renamed from lock_packed_refs()
 + commit_packed_refs(): use a staging file separate from the lockfile
 + commit_packed_refs(): report errors rather than dying
 + packed_ref_store: make class into a subclass of `ref_store`
 + packed-backend: new module for handling packed references
 + packed_read_raw_ref(): new function, replacing `resolve_packed_ref()`
 + packed_ref_store: support iteration
 + packed_peel_ref(): new function, extracted from `files_peel_ref()`
 + repack_without_refs(): take a `packed_ref_store *` parameter
 + get_packed_ref(): take a `packed_ref_store *` parameter
 + 

Re: [PATCH] Doc: clarify that pack-objects makes packs, plural

[Junio C Hamano]

Jonathan Tan  writes:

> The documentation for pack-objects describes that it creates "a packed
> archive of objects", which is confusing because it may create multiple
> packs if --max-pack-size is set. Update the documentation to clarify
> this.
>
> Signed-off-by: Jonathan Tan 
> ---
> It took me quite some time before I realized that pack-objects actually
> may write multiple packs, the opening lines of the doc confusing me.
> Here's a doc update.
> ---

I have a mixed feeling about this one.  

Yes, the command _can_ be told to split a packfile into multiple
with an option, but the actual benefit of doing so is rather
dubious.  On boxes with smaller address space, I thought windowed
mmap access into large packfiles work just fine.  I also think the
motivation behind the "max-size" thing was to split into smaller
pieces so that sneaker-netting on multiple CD-ROMs becomes easier or
something (silly) like that---there should be a more suitable tool
that is not specific to Git for such usecase, I would imagine.

So I am OK with "and writes either one or more" in the description,
but I'd prefer to see that "--max-pack-size" thing gets described as
an aberration, not a norm.

IOW,

 - I think the "NAME" part that gives a single line summary of what
   the command is about can and should stay as before.  A single
   archive is the norm, and we do not particularly recommend people
   to think it is a good idea to produce multiple packfiles.

 - The change in this patch for description part, which should give
   a fairly complete view of what it can do, is good.

 - The change for 'base-name' documentation that stresses that
   .pack/.idx come in pairs and share the same  is good.

 - There should be an update to say max-pack-size is not something
   normal users would ever want.

For the last one, perhaps something like this:


 Documentation/git-pack-objects.txt | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-pack-objects.txt 
b/Documentation/git-pack-objects.txt
index 8973510a41..3aa6234501 100644
--- a/Documentation/git-pack-objects.txt
+++ b/Documentation/git-pack-objects.txt
@@ -108,9 +108,13 @@ base-name::
is taken from the `pack.windowMemory` configuration variable.
 
 --max-pack-size=::
-   Maximum size of each output pack file. The size can be suffixed with
+   In unusual scenarios, you may not be able to create files
+   larger than certain size on your filesystem, and this option
+   can be used to tell the command to split the output packfile
+   into multiple independent packfiles and what the maximum
+   size of each packfile is. The size can be suffixed with
"k", "m", or "g". The minimum size allowed is limited to 1 MiB.
-   If specified, multiple packfiles may be created, which also
+   This option
prevents the creation of a bitmap index.
The default is unlimited, unless the config variable
`pack.packSizeLimit` is set.

Re: Cannot checkout after setting the eol attribute

[Ben Boeckel]

On Tue, Aug 22, 2017 at 21:13:18 +0200, Torsten Bögershausen wrote:
> When you set the text attribute (in your case "eol=crlf" implies text)
> then the file(s) -must- be nomalized and commited so that they have LF
> in the repo (technically speaking the index)

This seems like a special case that Git could detect and message about
somehow.

> This is what is written about the "eol=crlf" attribute:
>   This setting forces Git to normalize line endings for this
>   file on checkin and convert them to CRLF when the file is
>   checked out.
> And this is what is implemented in Git.

Yeah, I read the docs, but the oddities of reset not doing its job
wasn't clear from this sentence :) .

> Long story short:
> 
> The following would solve your problem:
>git init
>echo $'dos\r' > dos
>git add dos
>git commit -m "dos newlines"
>echo "dos -crlf" > .gitattributes
>git add .gitattributes
>git commit -m "add attributes"
>echo "dos eol=crlf" > .gitattributes
>git read-tree --empty   # Clean index, force re-scan of working directory

The fact that plumbing is necessary to dig yourself out of a hole of the
`eol` attribute changes points to something needing to be changed, even
if it's only documentation. Could Git detect this and message about it
somehow when `git reset` cannot fix the working tree? Or maybe it could
at least exit with failure instead of success?

--Ben

Re: [PATCH] Add 'raw' blob_plain link in history overview

[Job Snijders]

On Tue, Aug 22, 2017 at 12:22:43PM -0700, Junio C Hamano wrote:
> Job Snijders  writes:
> > Add 'raw' blob_plain link in history overview
> >
> > Reviewed-by: Giuseppe Bilotta 
> > Signed-off-by: Job Snijders 
> >
> > ---
> 
> Thanks; I somehow thought that your earlier one not just said what
> it does (twice---that is not needed) but why this change is useful,
> but that is lost in the patch description?

We often work with very large plain text files in our repositories and
found it friendlier to the users if we can click directly to the raw
version of such files.

Without this patch the workflow is to go to the history of a file, click
the 'blob' link, and then click the 'raw' link. If the file is large
(multiple megabytes) - rendering the html enveloppe to the blob can take
quite some time in the browser DOM rendering. 

This patch adds a 'raw' blob_plain link in history overview so you can
immediately click to the 'raw' version, saving time and improving
workflow.

Here is a screenshot of a gitweb instance with this patch applied:

http://instituut.net/~job/screenshots/b0f30e21eb64d5dda75ddabd.png

Kind regards,

Job

Re: Best way to check whether working tree matches a commit's tree

[Junio C Hamano]

Sebastian Schuberth  writes:

> Hi,
>
> I'd like to check whether my working tree exactly matches the tree of a given 
> commit. That is, there should not be any untracked, staged or modified files 
> (including ignored files).
>
> Currently, I'm doing this in two steps:
>
> - check for success and empty output of "git status --ignored --porcelain"
> - check that the output of "git rev-parse HEAD" matches the given commit
>
> While this works, it feels sub-optimal. Is there a better / smarter way?

I do not think so; you want three things to match and you have a way
to compare two things at a time.

By the way, I think your second check should compare

rev-parse HEAD^{tree} $that_commit^{tree}

as you are checking if the tree exactly matches.

Re: mk-dontmerge/size-t-on-next test failure

[Junio C Hamano]

Johannes Sixt  writes:

> I observe the test failure below in t0040-parse-options.sh. It bisects
> to 1a7909b25eb4ab3071ce4290115618e2582eadaa "Convert pack-objects to
> size_t". It looks like git_parse_size_t() needs a fix. This is on
> Windows, 32 bit. size_t, int and long are all 32 bits wide.

Thanks.  It is very much appreciated for people to peek at the
branch and try on different platforms that I (or Martin) do not use
regularly.

Re: [PATCH] Add 'raw' blob_plain link in history overview

[Junio C Hamano]

Job Snijders  writes:

> Add 'raw' blob_plain link in history overview
>
> Reviewed-by: Giuseppe Bilotta 
> Signed-off-by: Job Snijders 
>
> ---

Thanks; I somehow thought that your earlier one not just said what
it does (twice---that is not needed) but why this change is useful,
but that is lost in the patch description?

>  gitweb/gitweb.perl | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 9208f42ed..959f04b49 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -5967,6 +5967,9 @@ sub git_history_body {
> $cgi->a({-href => href(action=>"commitdiff", 
> hash=>$commit)}, "commitdiff");
>  
>   if ($ftype eq 'blob') {
> + print " | " .
> +   $cgi->a({-href => href(action=>"blob_plain", 
> hash_base=>$commit, file_name=>$file_name)}, "raw");
> +
>   my $blob_current = $file_hash;
>   my $blob_parent  = git_get_hash_by_path($commit, 
> $file_name);
>   if (defined $blob_current && defined $blob_parent &&

Re: Cannot checkout after setting the eol attribute

[Torsten Bögershausen]

On Tue, Aug 22, 2017 at 01:49:18PM -0400, Ben Boeckel wrote:
> Hi,
> 
> I specified the `eol` attribute on some files recently and the behavior
> of Git is very strange.
> 
> Here is the set of commands to set up the repository used for the
> discussion:
> 
> git init
> echo $'dos\r' > dos
> git add dos
> git commit -m "dos newlines"
> echo "dos -crlf" > .gitattributes
> git add .gitattributes
> git commit -m "add attributes"
> echo "dos eol=crlf" > .gitattributes
> git add .gitattributes
> git commit -m "set eol attribute instead"
> 
> The following behaviors are observed:
> 
>   - `git reset --hard` does not make the working directory clean; and
>   - `git rebase` gets *very* confused about the diffs in the working
> tree because `git stash` can't reset the working tree;
> 


> There are probably other oddities lingering about as well. If I commit
> what Git thinks is the difference, the diff (with invisibles made
> visible) is:
> 
> % git diff | cat -A
> diff --git a/dos b/dos$
> index fde2310..4723a1b 100644$
> --- a/dos$
> +++ b/dos$
> @@ -1 +1 @@$
> -dos^M$
> +dos$

Thanks for the goos example!
This is by design.

When you set the text attribute (in your case "eol=crlf" implies text)
then the file(s) -must- be nomalized and commited so that they have LF
in the repo (technically speaking the index)


This is what is written about the "eol=crlf" attribute:
This setting forces Git to normalize line endings for this
file on checkin and convert them to CRLF when the file is
checked out.
And this is what is implemented in Git.

Long story short:

The following would solve your problem:
   git init
   echo $'dos\r' > dos
   git add dos
   git commit -m "dos newlines"
   echo "dos -crlf" > .gitattributes
   git add .gitattributes
   git commit -m "add attributes"
   echo "dos eol=crlf" > .gitattributes
   git read-tree --empty   # Clean index, force re-scan of working directory
   git add dos
   git add .gitattributes
   git commit -m "set eol attribute instead"
   git ls-files --eol


> 
> Seen in 2.9.5 and 2.14.0.rc1.
> 
> --Ben

Re: [PATCH] pull: respect submodule update configuration

[Brandon Williams]

On 08/22, Stefan Beller wrote:
> On Tue, Aug 22, 2017 at 7:50 AM, Lars Schneider
>  wrote:
> >
> > OK. I change my scripts to use ".active" and it seems to work nicely.
> >
> > I noticed one oddity, though:
> >
> > If I clone a repo using `git clone --recursive ` then the local
> > Git config of the repo gets the following entry:
> >
> > [submodule]
> > active = .
> 
> bb62e0a99f (clone: teach --recurse-submodules to optionally take a
> pathspec, 2017-03-17) makes it clear that this is intentional for
> --recurse-submodules, but doesn't exactly state that --recurse will
> behave the same. The idea here is that at clone time you can already
> give
> 
> git clone --recurse=:(exclude)sub0  
> 
> and have your desired set of submodules there.
> Combined with the changes in the attr system, b0db704652
> (pathspec: allow querying for attributes, 2017-03-13)
> you could make up things like this:
> 
>   $ cat .gitattributes
>   /sub0 label0
>   /sub1
>   /sub2 label1 label2
>   /sub3 label1
>   /platform-specifc-subs/* label1 label2
> 
> and then get a clone via
> 
> git clone --recurse=:(attr:label2).  
> 
> for example. The labeling via the attributes allows for
> complex patterns, but a relatively easy command line, that you
> can share with coworkers.
> 
> > Is this intentional? Something in the git/git test harness seems to prevent
> > that. I was not able to write a test to replicate the issue.
> >
> > Any idea?
> 
> I do not seem to understand the perceived bug?
> The setting of submodule.active= seems intentional to me,
> but how would you not reproduce it? Maybe Brandon has an idea.
> 

When adding '.active' we wanted it to be as flexible as possible.  So
you can either use a pathspec with 'submodule.active' to catch multiple
submodules as being active or you can turn on/off individual submodules
with 'submodule..active' (this has precedent over the more general
'submodule.active' config).

The intent was if a user supplies --recurse-submodules (I believe i
removed the docs for --recursive in order to make the CLI more consistent
with other commands, so --recursive is just a synonym for
--recurse-submodules) then they clearly wanted all the submodules cloned
and checked out.  With the '.active' config the way to specify this
is to make 'submodule.active = .'.  In the old world every submodule
would need to have its URL copied into the config.  This way the config
is kept cleaner as it only has a single entry added.

As stefan mentioned you can specify a value for 'submodule.active' to
take as an arg to --recurse-submodules (the default being '.' or all
submodules) so you can do clever things like group submodules using
attributes, you can even repeat the flag to provided a more complex
pathspec.

Hopefully that answers your question :D

-- 
Brandon Williams

[PATCH] Doc: clarify that pack-objects makes packs, plural

[Jonathan Tan]

The documentation for pack-objects describes that it creates "a packed
archive of objects", which is confusing because it may create multiple
packs if --max-pack-size is set. Update the documentation to clarify
this.

Signed-off-by: Jonathan Tan 
---
It took me quite some time before I realized that pack-objects actually
may write multiple packs, the opening lines of the doc confusing me.
Here's a doc update.
---
 Documentation/git-pack-objects.txt | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-pack-objects.txt 
b/Documentation/git-pack-objects.txt
index 8973510a4..d8264ad57 100644
--- a/Documentation/git-pack-objects.txt
+++ b/Documentation/git-pack-objects.txt
@@ -3,7 +3,7 @@ git-pack-objects(1)
 
 NAME
 
-git-pack-objects - Create a packed archive of objects
+git-pack-objects - Create packed archives of objects
 
 
 SYNOPSIS
@@ -18,8 +18,9 @@ SYNOPSIS
 
 DESCRIPTION
 ---
-Reads list of objects from the standard input, and writes a packed
-archive with specified base-name, or to the standard output.
+Reads list of objects from the standard input, and writes either one or
+more packed archives with the specified base-name to disk, or a packed
+archive to the standard output.
 
 A packed archive is an efficient way to transfer a set of objects
 between two repositories as well as an access efficient archival
@@ -47,9 +48,9 @@ transport by their peers.
 OPTIONS
 ---
 base-name::
-   Write into a pair of files (.pack and .idx), using
+   Write into pairs of files (.pack and .idx), using
 to determine the name of the created file.
-   When this option is used, the two files are written in
+   When this option is used, the two files in a pair are written in
-.{pack,idx} files.   is a hash
based on the pack content and is written to the standard
output of the command.
-- 
2.14.1.342.g6490525c54-goog

Re: [RFC 0/3] imap-send curl tunnelling support

[Nicolas Morey-Chaisemartin]

This was sadly kind of expected...
I need to look for another way to handle this on Windows.

Thanks for the test

Nicolas

Le 22/08/2017 à 19:10, Johannes Sixt a écrit :
> Am 21.08.2017 um 09:27 schrieb Nicolas Morey-Chaisemartin:
>> (Sent a reply from my phone while out of town but couldn't find it so here 
>> it is again)
>>
>> It's available on my github:
>> https://github.com/nmorey/git/tree/dev/curl-tunnel
>>
>> The series had been stlighly changed since the patch were posted, mostly to 
>> add the proper ifdefs to handle older curl versions.
>
> This does not build for me on Windows due to a missing socketpair() function. 
> But I am working in an old environment, so I do not know whether this 
> statement has much value.
>
> -- Hannes

Re: [PATCH] repository: fix a sparse 'using integer a NULL pointer' warning

[Junio C Hamano]

René Scharfe  writes:

>> diff --git a/repository.c b/repository.c
>> index 01af20dee..ceef73614 100644
>> --- a/repository.c
>> +++ b/repository.c
>> @@ -5,7 +5,7 @@
>>   
>>   /* The main repository */
>>   static struct repository the_repo = {
>> -NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, 0, 0
>> +NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, NULL, 
>> 0, 0
>
> This line yells out "designated initializer" to me:
>
> + .index = _index
>
>>   };
>>   struct repository *the_repository = _repo;
>>   
>> 

Yes, but let's hold it off for a while, until at least what is
already in the tip of 'master' graduates to a released version at
the end of the current cycle.  We picked reasonably quiecent parts
of the codebase and implanted uses of a few C99 features to ensure
that we get complaints and requests for revert from people on exotic
platforms, so that we can back them out easily.

cbc0f81d ("strbuf: use designated initializers in STRBUF_INIT",
2017-07-10) does the designated initializer for struct members.

512f41cf ("clean.c: use designated initializer", 2017-07-14) does
the same for array elements.

In addition, e1327023 ("grep: refactor the concept of "grep source"
into an object", 2012-02-02) inadvertently introduced the use of
trailing comma in enum definition about 5 years ago, so we know that
one is safe to use.

Re: Submodule regression in 2.14?

[Stefan Beller]

On Tue, Aug 22, 2017 at 8:33 AM, Heiko Voigt  wrote:
> On Mon, Aug 21, 2017 at 09:42:54AM -0700, Stefan Beller wrote:
>> On Mon, Aug 21, 2017 at 9:05 AM, Heiko Voigt  wrote:
>> > On Fri, Aug 18, 2017 at 11:51:13PM -0700, Junio C Hamano wrote:
>> >> As long as we are talking about idealized future world (well, at
>> >> least an idea of somebody's "ideal", not necessarily shared by
>> >> everybody), I wonder if there is even any need to have commits in
>> >> submodules in such a world.  To realize such a "monorepo" world, you
>> >> might be better off allowing a gitlink in the superproject to
>> >> directly point at a tree object in a submodule repository (making
>> >> them physically a single repository is an optional implementation
>> >> detail I choose to ignore in this discussion).
>> >
>> > IMO this is one step to far. One main use of submodules are shared
>> > repositories that are used by many superprojects. The reason you want to
>> > have commits in the submodule are so that you can push them
>> > independently and all other users can pick up the changes. You could get
>> > by by Using the superproject commits for the submodule once you push or
>> > something but those do not necessarily make sense in the context of the
>> > submodule.
>> >
>> > So I think it is important that there are commits in the submodule so
>> > its history makes sense independently for others.
>> >
>> > Or how would you push out the history in the submodule in your idea?
>> > Maybe I am missing something? What would be your use case with gitlinks
>> > pointing to trees?
>>
>> Well there are still commits, but in the superproject the UX feels more
>> as if the submodules were special trees.
>
> Ah ok then I misunderstood. So they only feel like trees.
>
>> So if you want to interact with
>> the submodule specifically, you could do things like
>>
>> git add /path/inside/sub
>> # works seamlessly from the superproject tree
>
> Would that mean that we need to loosen/keep the requirement loose for a
> name from .gitmodules? I am asking because of my series for on-demand
> fetch of renamed submodules. For the full functionality I would require
> a name.
>
> Would that be in a scenario where the user would then e.g. push the
> submodule into the superproject?
>
> Ah wait I misunderstood again. You mean a file in an existing
> submodule right? Not adding submodule from a repository a user moved
> there?

Assuming the submodule is at  /path in this example, the effect of
that command could be achieved today via

git -C /path add inside/sub

(i.e. for git-add we "just" detect that there is a submodule boundary
and run the git-add inside the submodule)

>
>> git commit --submodule-commit-only
>> # When the flag is not give, you may get an editor
>> # asking for two commit messages, (sub+super)
>
> Or maybe something like
>
> git commit --submodule path/to/submodule

Yes. In todays UX, you do

git -C path/to/submodule commit

for the command that you proposed. For a plain
git-commit in the superproject, we could envision
this sequence of todays commands:

git -C submodule commit
git add submodule
git commit

>
> so the user can specify which submodule she wants. I first wrote it
> without the switch but but that collides with listing files which should
> be added. IMO this shorter option is also more intuitive to understand
> what it does (for this case).
>
>> git fetch --submodule
>> # When the flag is not given, we'd fetch superproject and
>> # on-demand
>
> Yes like above we should add the path to the submodule right?

yes.

>
>> # You feel the superproject is in the way?
>> git worktree add --for-submodule  ...
>> # The new submodule worktree puts the
>> # submodule only UX first. so it feels like its own
>> # repository, no need for specific flags.
>
> I am not sure I understand this one. What would that do? Put a worktree
> for submodule path/to/sub to ...?

Yes, and at "..." you would have the UX of the submodule being
its own repository, no interaction with the superproject.

>
> Overall I like the direction of these ideas.
>
> Cheers Heiko

Best way to check whether working tree matches a commit's tree

[Sebastian Schuberth]

Hi,

I'd like to check whether my working tree exactly matches the tree of a given 
commit. That is, there should not be any untracked, staged or modified files 
(including ignored files).

Currently, I'm doing this in two steps:

- check for success and empty output of "git status --ignored --porcelain"
- check that the output of "git rev-parse HEAD" matches the given commit

While this works, it feels sub-optimal. Is there a better / smarter way?

-- 
Sebastian Schuberth

Re: [PATCH] pull: respect submodule update configuration

[Stefan Beller]

On Tue, Aug 22, 2017 at 7:50 AM, Lars Schneider
 wrote:
>
> OK. I change my scripts to use ".active" and it seems to work nicely.
>
> I noticed one oddity, though:
>
> If I clone a repo using `git clone --recursive ` then the local
> Git config of the repo gets the following entry:
>
> [submodule]
> active = .

bb62e0a99f (clone: teach --recurse-submodules to optionally take a
pathspec, 2017-03-17) makes it clear that this is intentional for
--recurse-submodules, but doesn't exactly state that --recurse will
behave the same. The idea here is that at clone time you can already
give

git clone --recurse=:(exclude)sub0  

and have your desired set of submodules there.
Combined with the changes in the attr system, b0db704652
(pathspec: allow querying for attributes, 2017-03-13)
you could make up things like this:

  $ cat .gitattributes
  /sub0 label0
  /sub1
  /sub2 label1 label2
  /sub3 label1
  /platform-specifc-subs/* label1 label2

and then get a clone via

git clone --recurse=:(attr:label2).  

for example. The labeling via the attributes allows for
complex patterns, but a relatively easy command line, that you
can share with coworkers.

> Is this intentional? Something in the git/git test harness seems to prevent
> that. I was not able to write a test to replicate the issue.
>
> Any idea?

I do not seem to understand the perceived bug?
The setting of submodule.active= seems intentional to me,
but how would you not reproduce it? Maybe Brandon has an idea.

Thanks,
Stefan

Cannot checkout after setting the eol attribute

[Ben Boeckel]

Hi,

I specified the `eol` attribute on some files recently and the behavior
of Git is very strange.

Here is the set of commands to set up the repository used for the
discussion:

git init
echo $'dos\r' > dos
git add dos
git commit -m "dos newlines"
echo "dos -crlf" > .gitattributes
git add .gitattributes
git commit -m "add attributes"
echo "dos eol=crlf" > .gitattributes
git add .gitattributes
git commit -m "set eol attribute instead"

The following behaviors are observed:

  - `git reset --hard` does not make the working directory clean; and
  - `git rebase` gets *very* confused about the diffs in the working
tree because `git stash` can't reset the working tree;

There are probably other oddities lingering about as well. If I commit
what Git thinks is the difference, the diff (with invisibles made
visible) is:

% git diff | cat -A
diff --git a/dos b/dos$
index fde2310..4723a1b 100644$
--- a/dos$
+++ b/dos$
@@ -1 +1 @@$
-dos^M$
+dos$

Seen in 2.9.5 and 2.14.0.rc1.

--Ben

Re: [PATCH] repository: fix a sparse 'using integer a NULL pointer' warning

[René Scharfe]

Am 21.08.2017 um 17:48 schrieb Ramsay Jones:
> 
> Commit 67a9dfcc00 ("hash-algo: integrate hash algorithm support with
> repo setup", 21-08-2017) added a 'const struct git_hash_algo *hash_algo'
> field to the repository structure, without modifying the initializer
> of the 'the_repo' variable. This does not actually introduce a bug,
> since the '0' initializer for the 'ignore_env:1' bit-field is
> interpreted as a NULL pointer (hence the warning), and the final field
> (now with no initializer) receives a default '0'.
> 
> Signed-off-by: Ramsay Jones 
> ---
> 
> Hi Brian,
> 
> If you need to re-roll your 'bc/hash-algo' branch, could you please
> squash this into the relevant patch.
> 
> Thanks!
> 
> ATB,
> Ramsay Jones
> 
>   repository.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/repository.c b/repository.c
> index 01af20dee..ceef73614 100644
> --- a/repository.c
> +++ b/repository.c
> @@ -5,7 +5,7 @@
>   
>   /* The main repository */
>   static struct repository the_repo = {
> - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, 0, 0
> + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, NULL, 
> 0, 0

This line yells out "designated initializer" to me:

+   .index = _index

>   };
>   struct repository *the_repository = _repo;
>   
> 

Re: [RFC 0/3] imap-send curl tunnelling support

[Johannes Sixt]

Am 21.08.2017 um 09:27 schrieb Nicolas Morey-Chaisemartin:

(Sent a reply from my phone while out of town but couldn't find it so here it 
is again)

It's available on my github:
https://github.com/nmorey/git/tree/dev/curl-tunnel

The series had been stlighly changed since the patch were posted, mostly to add 
the proper ifdefs to handle older curl versions.


This does not build for me on Windows due to a missing socketpair() 
function. But I am working in an old environment, so I do not know 
whether this statement has much value.


-- Hannes

mk-dontmerge/size-t-on-next test failure

[Johannes Sixt]

I observe the test failure below in t0040-parse-options.sh. It bisects
to 1a7909b25eb4ab3071ce4290115618e2582eadaa "Convert pack-objects to
size_t". It looks like git_parse_size_t() needs a fix. This is on
Windows, 32 bit. size_t, int and long are all 32 bits wide.

expecting success:
check magnitude: 1073741824 -m 1g

ok 18 - OPT_MAGNITUDE() giga

expecting success:
check magnitude: 3221225472 -m 3g

error: switch `m' expects a non-negative integer value with an optional
k/m/g suffix
usage: test-parse-options 

--yes get a boolean
-D, --no-doubtbegins with 'no-'
-B, --no-fear be brave
-b, --boolean increment by one
-4, --or4 bitwise-or boolean with ...0100
--neg-or4 same as --no-or4

-i, --integer  get a integer
-j get a integer, too
-m, --magnitudeget a magnitude
--set23   set integer to 23
-t  get timestamp of 
-L, --length get length of 
-F, --file  set file to 

String options
-s, --string 
  get a string
--string2get another string
--st  get another string (pervert ordering)
-o   get another string
--list   add str to list

Magic arguments
--quuxmeans --quux
-NUM  set integer to NUM
+ same as -b
--ambiguous   positive ambiguity
--no-ambiguousnegative ambiguity

Standard options
--abbrev[=]use  digits to display SHA-1s
-v, --verbose be verbose
-n, --dry-run dry run
-q, --quiet   be quiet
--expect  expected output in the variable dump

not ok 19 - OPT_MAGNITUDE() 3giga
#
#   check magnitude: 3221225472 -m 3g
#

Re: [PATCH v2 1/3] Documentation/git-merge: explain --continue

[Junio C Hamano]

hIpPy  writes:

> I think 'git merge --continue' should be advertised more that 'git
> commit' as typically one is familiar with 'git rebase --continue' and
> 'git cherry-pick --continue'. I for a long time did not know I could
> also use 'git commit' to continue a merge but that's just me.

Perhaps.  "rebase" and "am" (and range pick "cherry-pick A..B") are
operations that works on more than one change, so it makes perfect
sense to have a way to say "I am done with this step. Please go on
and do the rest".

There is no "go on and do the rest" after resolving a conflicted
merge, as "merge" is, unlike the other ones to which "--continue"
legitimately has a reasonable meaning, an operation that does just
one thing.  From that point of view, "git merge --continue" is a
mistaken UI that shouldn't have happened.

Re: [PATCH v2 3/3] merge: save merge state earlier

[Junio C Hamano]

Michael J Gruber  writes:

>> Can squash ever be true in this function?
>> 
>> This function has two callsites: merge_trivial() and
>> finish_automerge().
>> 
>> I think merge_trivial() will not be called under "--squash", which
>> turns option_commit off and the only callsite of it is inside an
>> else-if clause that requres option_commit to be true.  You can do a
>> similar deduction around the "automerge_was_ok" variable to see if
>> finish_automerge() can be called when "--squash" is given; I suspect
>> the answer may be no.
>
> I'll go without the if, after more testing.

I was sort of expecting that tracing the control flow would give us
the definite answer and that would be much better than any amount of
testing.

In any case, I wasn't even suggesting to remove "if".  It might even
be worth doing

if (squash)
BUG("the control must not reach here under --squash");
write_emrge_heads(...);

if we know the control does not have to reach with "--squash" in
today's code, so that future careless refactoring does not break
this fix.

[PATCH v3 4/4] imap-send: use curl by default

[Nicolas Morey-Chaisemartin]

Now that curl is enable by default, use the curl implementation
for imap too.
The goal is to validate feature parity between the legacy and
the curl implementation, deprecate thee legacy implementation
later on and in the long term, hopefully drop it altogether.

Signed-off-by: Nicolas Morey-Chaisemartin 
---
 imap-send.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index a74d011a9..58c191704 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -35,11 +35,11 @@ typedef void *SSL;
 #include "http.h"
 #endif
 
-#if defined(USE_CURL_FOR_IMAP_SEND) && defined(NO_OPENSSL)
-/* only available option */
+#if defined(USE_CURL_FOR_IMAP_SEND)
+/* Always default to curl if it's available. */
 #define USE_CURL_DEFAULT 1
 #else
-/* strictly opt in */
+/* We don't have curl, so continue to use the historical implementation */
 #define USE_CURL_DEFAULT 0
 #endif
 
-- 
2.14.0.1.gd9597ce13

Re: What's cooking in git.git (Aug 2017, #04; Fri, 18)

[Junio C Hamano]

Duy Nguyen  writes:

> On Sat, Aug 19, 2017 at 4:26 AM, Junio C Hamano  wrote:
>> [Discarded]
>>
>> * nd/prune-in-worktree (2017-04-24) 12 commits
>>  . rev-list: expose and document --single-worktree
>>  . revision.c: --reflog add HEAD reflog from all worktrees
>>  . files-backend: make reflog iterator go through per-worktree reflog
>>  . revision.c: --all adds HEAD from all worktrees
>>  . refs: remove dead for_each_*_submodule()
>>  . revision.c: use refs_for_each*() instead of for_each_*_submodule()
>>  . refs: add refs_head_ref()
>>  . refs: move submodule slash stripping code to get_submodule_ref_store
>>  . refs.c: refactor get_submodule_ref_store(), share common free block
>>  . revision.c: --indexed-objects add objects from all worktrees
>>  . revision.c: refactor add_index_objects_to_pending()
>>  . revision.h: new flag in struct rev_info wrt. worktree-related refs
>>
>>  "git gc" and friends when multiple worktrees are used off of a
>>  single repository did not consider the index and per-worktree refs
>>  of other worktrees as the root for reachability traversal, making
>>  objects that are in use only in other worktrees to be subject to
>>  garbage collection.
>
> I'm back and will try to continue this. Is it discarded because of
> lack of progress, or because the problem is already fixed some other
> way? A quick "git log --oneline" on important files has not revealed
> anything.

Welcome back. 

I ejected it out of 'pu' due to inactivity and possibly because I
saw some conflicts with topics that were making progress back then.
I do not offhand know if the old one still merges cleanly to 'pu',
but it certainly wasn't because the topic was deemed unnecessary.

Thanks.

[PATCH v3 3/4] imap_send: setup_curl: retreive credentials if not set in config file

[Nicolas Morey-Chaisemartin]

Up to this point, the curl mode only supported getting the username
and password from the gitconfig file while the legacy mode could also
fetch them using the credential API.

Signed-off-by: Nicolas Morey-Chaisemartin 
---
 imap-send.c | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 448a4a0b3..a74d011a9 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1398,7 +1398,7 @@ static int append_msgs_to_imap(struct imap_server_conf 
*server,
 }
 
 #ifdef USE_CURL_FOR_IMAP_SEND
-static CURL *setup_curl(struct imap_server_conf *srvc)
+static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred)
 {
CURL *curl;
struct strbuf path = STRBUF_INIT;
@@ -1411,6 +1411,7 @@ static CURL *setup_curl(struct imap_server_conf *srvc)
if (!curl)
die("curl_easy_init failed");
 
+   server_fill_credential(, cred);
curl_easy_setopt(curl, CURLOPT_USERNAME, server.user);
curl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);
 
@@ -1460,8 +1461,9 @@ static int curl_append_msgs_to_imap(struct 
imap_server_conf *server,
struct buffer msgbuf = { STRBUF_INIT, 0 };
CURL *curl;
CURLcode res = CURLE_OK;
+   struct credential cred = CREDENTIAL_INIT;
 
-   curl = setup_curl(server);
+   curl = setup_curl(server, );
curl_easy_setopt(curl, CURLOPT_READDATA, );
 
fprintf(stderr, "sending %d message%s\n", total, (total != 1) ? "s" : 
"");
@@ -1496,6 +1498,18 @@ static int curl_append_msgs_to_imap(struct 
imap_server_conf *server,
curl_easy_cleanup(curl);
curl_global_cleanup();
 
+   if (cred.username)
+   if (res == CURLE_OK)
+   credential_approve();
+#if LIBCURL_VERSION_NUM >= 0x070d01
+   else if (res == CURLE_LOGIN_DENIED)
+#else
+   else
+#endif
+   credential_reject();
+
+   credential_clear();
+
return res == CURLE_OK;
 }
 #endif
-- 
2.14.0.1.gd9597ce13

[PATCH v3 2/4] imap-send: add wrapper to get server credentials if needed

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin 
---
 imap-send.c | 34 --
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 09f29ea95..448a4a0b3 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -926,6 +926,25 @@ static int auth_cram_md5(struct imap_store *ctx, struct 
imap_cmd *cmd, const cha
return 0;
 }
 
+static void server_fill_credential(struct imap_server_conf *srvc, struct 
credential *cred)
+{
+   if (srvc->user && srvc->pass)
+   return;
+
+   cred->protocol = xstrdup(srvc->use_ssl ? "imaps" : "imap");
+   cred->host = xstrdup(srvc->host);
+
+   cred->username = xstrdup_or_null(srvc->user);
+   cred->password = xstrdup_or_null(srvc->pass);
+
+   credential_fill(cred);
+
+   if (!srvc->user)
+   srvc->user = xstrdup(cred->username);
+   if (!srvc->pass)
+   srvc->pass = xstrdup(cred->password);
+}
+
 static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char 
*folder)
 {
struct credential cred = CREDENTIAL_INIT;
@@ -1078,20 +1097,7 @@ static struct imap_store *imap_open_store(struct 
imap_server_conf *srvc, char *f
}
 #endif
imap_info("Logging in...\n");
-   if (!srvc->user || !srvc->pass) {
-   cred.protocol = xstrdup(srvc->use_ssl ? "imaps" : 
"imap");
-   cred.host = xstrdup(srvc->host);
-
-   cred.username = xstrdup_or_null(srvc->user);
-   cred.password = xstrdup_or_null(srvc->pass);
-
-   credential_fill();
-
-   if (!srvc->user)
-   srvc->user = xstrdup(cred.username);
-   if (!srvc->pass)
-   srvc->pass = xstrdup(cred.password);
-   }
+   server_fill_credential(srvc, );
 
if (srvc->auth_method) {
struct imap_cmd_cb cb;
-- 
2.14.0.1.gd9597ce13

[PATCH v3 1/4] imap-send: return with error if curl failed

[Nicolas Morey-Chaisemartin]

curl_append_msgs_to_imap always returned 0, whether curl failed or not.
Return a proper status so git imap-send will exit with an error code
if womething wrong happened.

Signed-off-by: Nicolas Morey-Chaisemartin 
---
 imap-send.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/imap-send.c b/imap-send.c
index b2d0b849b..09f29ea95 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1490,7 +1490,7 @@ static int curl_append_msgs_to_imap(struct 
imap_server_conf *server,
curl_easy_cleanup(curl);
curl_global_cleanup();
 
-   return 0;
+   return res == CURLE_OK;
 }
 #endif
 
-- 
2.14.0.1.gd9597ce13

[PATCH v3 0/4] imap-send: Fix and enable curl by default

[Nicolas Morey-Chaisemartin]

Changes since v2:
Patch 3 reject credit when curl failed:
 - when a login error  is returned (curl >= 7.13.1)
 - for any curl error in older versions

Nicolas Morey-Chaisemartin (4):
  imap-send: return with error if curl failed
  imap-send: add wrapper to get server credentials if needed
  imap_send: setup_curl: retreive credentials if not set in config file
  imap-send: use curl by default

 imap-send.c | 60 
 1 file changed, 40 insertions(+), 20 deletions(-)

-- 
2.14.0.1.gd9597ce13

Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase

[Junio C Hamano]

Phillip Wood  writes:

>> In other words, instead of
>> 
>>  git add -u && git rebase --continue
>> 
>> you would want a quicker way to say
>> 
>>  git rebase --continue $something_here 
>
> Exactly
> ...
> rebase --continue -a
>
> behaves like commit -a in that it commits all updated tracked files and
> does not take pathspecs.

Hmph, but what 'a' in "commit -a" wants to convey is "all", and in
the context of "rebase" we want to avoid saying "all".  A user can
be confused into thinking "all" refers to "all subsequent commits" 
not "all paths conflicted".

Besides, with the $something_here option, the user is explicitly
telling us that the user finisished the resolution of conflicts left
in the working tree.  There is nothing "auto" about it.

> Did you have any further thoughts on what checks if any this new option
> should make to avoid staging obviously unresolved files?

The more I think about this, the more I am convinced that it is a
very bad idea to give such a $something_here option only to "rebase".

The standard flow for any operation that could stop in the middle
because the command needs help from the user with conflicts that
cannot be mechanically resolved is

 (1) write out conflicted state in the working tree, and mark these
 paths in the index by leaving them in higher stages
 (i.e. "ls-files -u" would list them);

 (2) the user edits them and marks the ones that are now resolved;

 (3) the user tells us to "continue".

and this is not limited to "rebase".  "cherry-pick", "am", and
"revert" all share the above, and even "merge" (which is a single
commit operation to which "continue" in its original meaning---to
tell us the user is done with this step and wants us to go on
processing further commits or patches---does not quite apply) does.

And "rebase" is an oddball in that it is the only command that we
could deduce which files in the working tree should be "add"ed, i.e.
"all the files that are different from HEAD".  All others allow
users to begin in a dirty working tree (they only require the index
to be clean), so your 

git [cherry-pick|am|revert] --continue $something_here

cannot be "git add -u && git [that command] --continue".  Blindly
taking any and all files that have modifications from HEAD will
produce a wrong result.

You cannot even sanely solve that by first recording the paths that
are conflicted before giving control back to the user and limit the
"add" to them, i.e. doing "git add" only for paths that appear in
"git ls-files -u" output would not catch additional changes the was
needed in files that did not initially conflict (i.e. "evil merge"
will have to modify a file that was not involved in the mechanical
part of the conflict), because out of the files that are dirty in
the working tree, some are evil merge resolutions and others are
ones that were dirty from the beginning.

The only reason "git rebase" can get away without having to worry
about all of the above is because it insists that the working tree
is clean before it begins.  In the ideal world, however, we would
want to lift that and allow it to start in a working tree that has
an unfinished local modification that does not interfere with the
rebase, just like all other operations that could stop upon a
conflict.  And when that happens, your $something_here option to
"git rebase --continue" will have to worry about it, too.

So...

Re: Submodule regression in 2.14?

[Heiko Voigt]

On Mon, Aug 21, 2017 at 09:48:51AM -0700, Junio C Hamano wrote:
> Heiko Voigt  writes:
> 
> > So I think it is important that there are commits in the submodule so
> > its history makes sense independently for others.
> 
> I was trying to push the "just like trees" to the logical conclusion
> in order to see see if it leads to an absurd conclusions, or some
> useful scenario.  I do not necessarily subscribed to Jonathan's
> "vision" (I do not object to it as one possible mode of operation,
> either).
> 
> > Or how would you push out the history in the submodule in your idea?
> > Maybe I am missing something? What would be your use case with gitlinks
> > pointing to trees?
> 
> Not my idea.  But Stefan's message I was responding to said that
> object database for the superproject is the same as and mixed with
> object databases for the submodules, so it is plausible that push
> always happens at the superproject, and a mechanism to be invented
> (I mentioned the need for it in the message you are responding to)
> to enumerate all the commits bound from submodules to a range of
> superproject's commits would identify these trees to be pushed out.
> 
> In essense, "just like trees" folks want to use the gitlinks in the
> superproject to only specify the tree from the submodule project
> that should sit there.  And my point is that such a world view would
> lead to no need for branches in the submodule repository, no need
> for commits in the submodule repository, and no need for history in
> the submodule repository.  Which may or may not be a bad thing.

The problem I see here is: How do we seperate the submodule from the
superproject? Without the history (commits and trees) of the
superproject the submodule trees do not make sense anymore. The reason
users have submodules is because the projects are seperate somehow. With
just trees in the submodule it becomes tied quite tightly to the
superproject, IMO.

One step further: Why use gitlinks to point to trees? Let's just use
normal tree links instead, we already have that implemented :)

Cheers Heiko

Re: Submodule regression in 2.14?

[Heiko Voigt]

On Mon, Aug 21, 2017 at 09:42:54AM -0700, Stefan Beller wrote:
> On Mon, Aug 21, 2017 at 9:05 AM, Heiko Voigt  wrote:
> > On Fri, Aug 18, 2017 at 11:51:13PM -0700, Junio C Hamano wrote:
> >> As long as we are talking about idealized future world (well, at
> >> least an idea of somebody's "ideal", not necessarily shared by
> >> everybody), I wonder if there is even any need to have commits in
> >> submodules in such a world.  To realize such a "monorepo" world, you
> >> might be better off allowing a gitlink in the superproject to
> >> directly point at a tree object in a submodule repository (making
> >> them physically a single repository is an optional implementation
> >> detail I choose to ignore in this discussion).
> >
> > IMO this is one step to far. One main use of submodules are shared
> > repositories that are used by many superprojects. The reason you want to
> > have commits in the submodule are so that you can push them
> > independently and all other users can pick up the changes. You could get
> > by by Using the superproject commits for the submodule once you push or
> > something but those do not necessarily make sense in the context of the
> > submodule.
> >
> > So I think it is important that there are commits in the submodule so
> > its history makes sense independently for others.
> >
> > Or how would you push out the history in the submodule in your idea?
> > Maybe I am missing something? What would be your use case with gitlinks
> > pointing to trees?
> 
> Well there are still commits, but in the superproject the UX feels more
> as if the submodules were special trees.

Ah ok then I misunderstood. So they only feel like trees.

> So if you want to interact with
> the submodule specifically, you could do things like
> 
> git add /path/inside/sub
> # works seamlessly from the superproject tree

Would that mean that we need to loosen/keep the requirement loose for a
name from .gitmodules? I am asking because of my series for on-demand
fetch of renamed submodules. For the full functionality I would require
a name.

Would that be in a scenario where the user would then e.g. push the
submodule into the superproject?

Ah wait I misunderstood again. You mean a file in an existing
submodule right? Not adding submodule from a repository a user moved
there?

> git commit --submodule-commit-only
> # When the flag is not give, you may get an editor
> # asking for two commit messages, (sub+super)

Or maybe something like

git commit --submodule path/to/submodule

so the user can specify which submodule she wants. I first wrote it
without the switch but but that collides with listing files which should
be added. IMO this shorter option is also more intuitive to understand
what it does (for this case).

> git fetch --submodule
> # When the flag is not given, we'd fetch superproject and
> # on-demand

Yes like above we should add the path to the submodule right?

> # You feel the superproject is in the way?
> git worktree add --for-submodule  ...
> # The new submodule worktree puts the
> # submodule only UX first. so it feels like its own
> # repository, no need for specific flags.

I am not sure I understand this one. What would that do? Put a worktree
for submodule path/to/sub to ...?

Overall I like the direction of these ideas.

Cheers Heiko

Re: [PATCH 0/1] Add stash entry count summary to short status output

[Sonny Michaud]

I am just bumping this thread; I presume there is something amiss with 
my submissions, so if someone can let me know how to fix any issues, I 
will gladly re-submit the patch.


Thanks!

- Sonny

Re: [PATCH v2 1/3] Documentation/git-merge: explain --continue

[hIpPy]

I think 'git merge --continue' should be advertised more that 'git
commit' as typically one is familiar with 'git rebase --continue' and
'git cherry-pick --continue'. I for a long time did not know I could
also use 'git commit' to continue a merge but that's just me. Now,
'git commit' is easier to remember if it works in all cases (merge,
rebase, cherry-pick).

RM


On Tue, Aug 22, 2017 at 3:06 AM, Martin Ågren  wrote:
> On 22 August 2017 at 11:26, Michael J Gruber  wrote:
>> Martin Ågren venit, vidit, dixit 21.08.2017 18:43:
>>> On 21 August 2017 at 14:53, Michael J Gruber  wrote:
 Currently, 'git merge --continue' is mentioned but not explained.

 Explain it.

 Signed-off-by: Michael J Gruber 
 ---
  Documentation/git-merge.txt | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

 diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
 index 6b308ab6d0..615e6bacde 100644
 --- a/Documentation/git-merge.txt
 +++ b/Documentation/git-merge.txt
 @@ -288,7 +288,10 @@ After seeing a conflict, you can do two things:

   * Resolve the conflicts.  Git will mark the conflicts in
 the working tree.  Edit the files into shape and
 -   'git add' them to the index.  Use 'git commit' to seal the deal.
 +   'git add' them to the index.  Use 'git commit' or
 +   'git merge --continue' to seal the deal. The latter command
 +   checks whether there is a (interrupted) merge in progress
 +   before calling 'git commit'.

  You can work through the conflict with a number of tools:
>>>
>>> There are actually two things going on here. First, this mentions git
>>> merge --continue. Second, it explains what that command does. But the
>>> latter is done earlier (not exactly like here, but still).
>>
>> I didn't see that explained in the man page at all - on the contrary, I
>> only saw a forward reference (see section...), but then only an
>> explanation of what "resolving" means (including the "git commit"-step).
>> It is unclear to me from the man page which steps of "resolving" the
>> command "git merge --continue" does - you could think it does "git
>> commit -a", for example.
>
> That's very true, and your change helps immensely. I thought that once
> git merge --continue was mentioned, e.g.,
>
> Use 'git commit' or 'git merge --continue' to seal the deal.
>
> or
>
> Use 'git commit' to conclude (you can also say 'git merge
> --continue').
>
> then things are in some sense "complete". But you might be right that
> further stressing that the latter is basically an alias helps avoid some
> confusion. "Oh, great, so now I have two commands to choose from -- which
> one should I be using?" :-)
>
> Martin

Re: t5551 hangs ?

[Santiago Torres]

> No concurrency.
> That's what I do: ./t5551-http-fetch-smart.sh

Oh ok! I was just trying to weed out what could out of place. Just to
make sure, your old compute is also running debian 8 latest? 
> 
> > - You probably want to see the version of apache this is running/etc.
> 
> The one that comes with Debian -
> I am not an expert here, what is it that is interesting ?

I'm not sure, but sometimes it's helpful to look at changelogs between
versions of packages to get a poiner as to what could be wrong. I wonder
if it's apache the one that's failing on this case. Probably a paste of
dpkg-query --show apache would help me try to reproduce..

> 
> > - What happens if you kill the apache processes? 
> 
> I am left with these processes:
> /home/blabla/pu/git -C too-many-refs fetch -q --tags
> /home/blabla/pu/git-remote-http origin http://127.0.0.1:5551/smart/repo.git
> 

So it's probably not an issue with apache, as the socket should be
hopefully closed gracefully.

> > 
> > I can't reproduce on my side, but let me see if I can dig a little into
> > it.
> 
> Thanks, more tips or things I can do are welcome.
> t5551 seems to be flaky - from time to time.
> It seems that I have it reproducable unstable, so if someone has more
> ideas, please.

I'm still unable to reproduce. Do you think you can enable GIT_TRACE,
GIT_TRACE_PACK and GIT_TRACE_CURL and pastebin/paste what you see?

Cheers!
-Santiago.


signature.asc
Description: PGP signature

Re: [PATCH] pull: respect submodule update configuration

[Lars Schneider]

> On 21 Aug 2017, at 20:21, Brandon Williams  wrote:
> 
> On 08/21, Stefan Beller wrote:
>> On Mon, Aug 21, 2017 at 10:20 AM, Lars Schneider
>>  wrote:
>>> 
 On 21 Aug 2017, at 18:55, Stefan Beller  wrote:
 
 On Mon, Aug 21, 2017 at 9:20 AM, Heiko Voigt  wrote:
 
>> So I am a bit curious to learn which part of this change you dislike
>> and why.
> 
> I am also curious. Isn't this the same strategy we are using in other
> places?
> 
 
 I dislike it because the UX feels crude.  When reading the documentation,
 it seems to me as if submodule. can be one of the following
 
   (none, checkout, rebase, merge, !)
 
 This is perfect for "submodule-update", whose primary goal is
 to update submodules *somehow*. However other commands
 
   git rebase --recurse
   git merge --recurse
   git checkout --recurse
 
 have a different primary mode of operation (note how their name
 is one of the modes from the set above), so it may get confusing
 for a user.
 
 'none'  and '!' seem like they would be okay
 for any of the commands above but then:
 
   git config submodule..update "!..."
   git reset --hard --recurse
   git status
   # submodule is reported, because "!..." did not 'reset'.
 
 Anyway. That dislike is just a minor gut feeling about the UX/UI
 being horrible. I wrote the patch to keep the conversation going,
 and if it fixes Lars problem, let's take it for now.
>>> 
>>> Well, I need just a way to disable certain Submodules completely.
>>> If you show me how "git config --local submodule.sub.active false"
>>> works then I don't need this patch.
> 
> Yeah if you want to completely disable a submodule (as in not even check
> it out) then setting .active to false would do that.  But as stefan
> pointed out and IIRC 'submodule update --init' with no pathspec sets all
> submodules to be active.  Perhaps it should only init submodules who
> don't already have an explicit active flag set.

OK. I change my scripts to use ".active" and it seems to work nicely.

I noticed one oddity, though:

If I clone a repo using `git clone --recursive ` then the local
Git config of the repo gets the following entry:

[submodule]
active = .

Is this intentional? Something in the git/git test harness seems to prevent
that. I was not able to write a test to replicate the issue.

Any idea?

Thanks,
Lars

Re: t5551 hangs ?

[Torsten Bögershausen]

On Tue, Aug 22, 2017 at 01:26:25AM -0400, Santiago Torres wrote:
> Hello, Torsten. 
> 
> On Tue, Aug 22, 2017 at 07:10:59AM +0200, Torsten Bögershausen wrote:
> > Hej,
> > I found 2 threads about hanging t5551, one in 2016, and one question
> > from Junio somewhen in 2017.
> > I have it reproducable here:
> > - Debian 8, 64 bit
> > - SSD
> > - Half-modern processor ;-)
> 
> I don't think the SSD/regular disk have anything to do with it. 
> 
> - Are you running tests concurrently? (e.g., with -j x)
> - What happens if you run the test individually (i.e., cd t and
>   ./t5551...)

No concurrency.
That's what I do: ./t5551-http-fetch-smart.sh

> - You probably want to see the version of apache this is running/etc.

The one that comes with Debian -
I am not an expert here, what is it that is interesting ?

> - What happens if you kill the apache processes? 

I am left with these processes:
/home/blabla/pu/git -C too-many-refs fetch -q --tags
/home/blabla/pu/git-remote-http origin http://127.0.0.1:5551/smart/repo.git

> 
> I can't reproduce on my side, but let me see if I can dig a little into
> it.

Thanks, more tips or things I can do are welcome.
t5551 seems to be flaky - from time to time.
It seems that I have it reproducable unstable, so if someone has more
ideas, please.

> 
> Cheers!
> -Santiago.

Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase

[Phillip Wood]

On 21/08/17 23:41, Junio C Hamano wrote:
> Phillip Wood  writes:
> 
>> ... I prefer
>> having to pass --autostage with --continue so that it is a concious
>> decision by the user to stage unstaged changes when they continue rather
>> than rebase just doing it each time it continues.
> 
> In other words, instead of
> 
>   git add -u && git rebase --continue
> 
> you would want a quicker way to say
> 
>   git rebase --continue $something_here 

Exactly

> If that is the case, that is understandable to me.  Is the "-u" (I
> think "git add -u" is short for "--update" but I didn't check) taken
> as a valid option to "git rebase"?  If not, that $something_here could
> be "-u".

At the moment $something_else is -a/--autostage but -u/--update (I've
checked the add man page and you're right) could be good as well.

rebase --continue -a

behaves like commit -a in that it commits all updated tracked files and
does not take pathspecs, if we go for -u then there is a difference with
'git add -u' as that can take an optional pathspec.


Did you have any further thoughts on what checks if any this new option
should make to avoid staging obviously unresolved files?

> Thanks for pinging the thread; otherwise I would have forgotten,
> especially because not many other people were involved in the
> discussion to begin with.

Yes it would be interesting to hear if this would be useful for others too.

Best Wishes

Phillip

Re: [PATCH v2 1/3] Documentation/git-merge: explain --continue

[Martin Ågren]

On 22 August 2017 at 11:26, Michael J Gruber  wrote:
> Martin Ågren venit, vidit, dixit 21.08.2017 18:43:
>> On 21 August 2017 at 14:53, Michael J Gruber  wrote:
>>> Currently, 'git merge --continue' is mentioned but not explained.
>>>
>>> Explain it.
>>>
>>> Signed-off-by: Michael J Gruber 
>>> ---
>>>  Documentation/git-merge.txt | 5 -
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
>>> index 6b308ab6d0..615e6bacde 100644
>>> --- a/Documentation/git-merge.txt
>>> +++ b/Documentation/git-merge.txt
>>> @@ -288,7 +288,10 @@ After seeing a conflict, you can do two things:
>>>
>>>   * Resolve the conflicts.  Git will mark the conflicts in
>>> the working tree.  Edit the files into shape and
>>> -   'git add' them to the index.  Use 'git commit' to seal the deal.
>>> +   'git add' them to the index.  Use 'git commit' or
>>> +   'git merge --continue' to seal the deal. The latter command
>>> +   checks whether there is a (interrupted) merge in progress
>>> +   before calling 'git commit'.
>>>
>>>  You can work through the conflict with a number of tools:
>>
>> There are actually two things going on here. First, this mentions git
>> merge --continue. Second, it explains what that command does. But the
>> latter is done earlier (not exactly like here, but still).
>
> I didn't see that explained in the man page at all - on the contrary, I
> only saw a forward reference (see section...), but then only an
> explanation of what "resolving" means (including the "git commit"-step).
> It is unclear to me from the man page which steps of "resolving" the
> command "git merge --continue" does - you could think it does "git
> commit -a", for example.

That's very true, and your change helps immensely. I thought that once
git merge --continue was mentioned, e.g.,

Use 'git commit' or 'git merge --continue' to seal the deal.

or

Use 'git commit' to conclude (you can also say 'git merge
--continue').

then things are in some sense "complete". But you might be right that
further stressing that the latter is basically an alias helps avoid some
confusion. "Oh, great, so now I have two commands to choose from -- which
one should I be using?" :-)

Martin

Re: [PATCH v2 3/3] merge: save merge state earlier

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 22.08.2017 02:38:
> Michael J Gruber  writes:
> 
>>  static void prepare_to_commit(struct commit_list *remoteheads)
>>  {
>>  struct strbuf msg = STRBUF_INIT;
>> @@ -767,6 +768,8 @@ static void prepare_to_commit(struct commit_list 
>> *remoteheads)
>>  strbuf_commented_addf(, _(merge_editor_comment), 
>> comment_line_char);
>>  if (signoff)
>>  append_signoff(, ignore_non_trailer(msg.buf, msg.len), 0);
>> +if (!squash)
>> +write_merge_heads(remoteheads);
>>  write_file_buf(git_path_merge_msg(), msg.buf, msg.len);
>>  if (run_commit_hook(0 < option_edit, get_index_file(), 
>> "prepare-commit-msg",
>>  git_path_merge_msg(), "merge", NULL))
> 
> I can understand that you would never want to write out MERGE_HEAD
> while squashing, but I somehow think it would be a bug in the caller
> to call prepare_to_commit(), whose point is to prepare the merge
> message to be recorded in the resulting merge commit, when the user
> gave us the "--squash" option, which is an explicit instruction that
> the user does not want the merge commit the message is used.

That sounds reasonable. I vaguely remember a failing test for an earlier
version that I tried, but that was before the "split".

> Can squash ever be true in this function?
> 
> This function has two callsites: merge_trivial() and
> finish_automerge().
> 
> I think merge_trivial() will not be called under "--squash", which
> turns option_commit off and the only callsite of it is inside an
> else-if clause that requres option_commit to be true.  You can do a
> similar deduction around the "automerge_was_ok" variable to see if
> finish_automerge() can be called when "--squash" is given; I suspect
> the answer may be no.

I'll go without the if, after more testing.

>> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
>> index 2ebda509ac..80194b79f9 100755
>> --- a/t/t7600-merge.sh
>> +++ b/t/t7600-merge.sh
>> @@ -774,4 +774,19 @@ test_expect_success 'merge can be completed with 
>> --continue' '
>>  verify_parents $c0 $c1
>>  '
>>  
>> +write_script .git/FAKE_EDITOR <> +# kill -TERM command added below.
>> +EOF
>> +
>> +test_expect_success EXECKEEPSPID 'killed merge can be completed with 
>> --continue' '
>> +git reset --hard c0 &&
>> +! "$SHELL_PATH" -c '\''
>> +  echo kill -TERM $$ >> .git/FAKE_EDITOR
>> +  GIT_EDITOR=.git/FAKE_EDITOR
>> +  export GIT_EDITOR
>> +  exec git merge --no-ff --edit c1'\'' &&
> 
> This is a tricky construct.  You "reserve" a process ID by using a
> shell, arrange it to be killed and then using "exec" to make it the
> "git merge" program to be killed.  I kind of like the convolutedness.

That is from t7502. Sorry for hiding that note in the cover letter, I
should put it into 3/3's message or a test comment.

When testing, I simply used "git merge... &" and "ps" or "jobs" to know
which process to kill. Apparantly, the above is the most portable way to
script that. t7502 went through a few iterations to ensure this.

>> +git merge --continue &&
>> +verify_parents $c0 $c1
>> +'
>> +
>>  test_done

Re: [PATCH v2 1/3] Documentation/git-merge: explain --continue

[Michael J Gruber]

Martin Ågren venit, vidit, dixit 21.08.2017 18:43:
> On 21 August 2017 at 14:53, Michael J Gruber  wrote:
>> Currently, 'git merge --continue' is mentioned but not explained.
>>
>> Explain it.
>>
>> Signed-off-by: Michael J Gruber 
>> ---
>>  Documentation/git-merge.txt | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
>> index 6b308ab6d0..615e6bacde 100644
>> --- a/Documentation/git-merge.txt
>> +++ b/Documentation/git-merge.txt
>> @@ -288,7 +288,10 @@ After seeing a conflict, you can do two things:
>>
>>   * Resolve the conflicts.  Git will mark the conflicts in
>> the working tree.  Edit the files into shape and
>> -   'git add' them to the index.  Use 'git commit' to seal the deal.
>> +   'git add' them to the index.  Use 'git commit' or
>> +   'git merge --continue' to seal the deal. The latter command
>> +   checks whether there is a (interrupted) merge in progress
>> +   before calling 'git commit'.
>>
>>  You can work through the conflict with a number of tools:
> 
> There are actually two things going on here. First, this mentions git
> merge --continue. Second, it explains what that command does. But the
> latter is done earlier (not exactly like here, but still).

I didn't see that explained in the man page at all - on the contrary, I
only saw a forward reference (see section...), but then only an
explanation of what "resolving" means (including the "git commit"-step).
It is unclear to me from the man page which steps of "resolving" the
command "git merge --continue" does - you could think it does "git
commit -a", for example.

> When git merge --continue originally appeared, this part of the docs was
> discussed briefly. Maybe interesting:
> 
> https://public-inbox.org/git/xmqq60mn671x@gitster.mtv.corp.google.com/
> 
> Martin
> 

Re: What's cooking in git.git (Aug 2017, #04; Fri, 18)

[Duy Nguyen]

On Sat, Aug 19, 2017 at 4:26 AM, Junio C Hamano  wrote:
> [Discarded]
>
> * nd/prune-in-worktree (2017-04-24) 12 commits
>  . rev-list: expose and document --single-worktree
>  . revision.c: --reflog add HEAD reflog from all worktrees
>  . files-backend: make reflog iterator go through per-worktree reflog
>  . revision.c: --all adds HEAD from all worktrees
>  . refs: remove dead for_each_*_submodule()
>  . revision.c: use refs_for_each*() instead of for_each_*_submodule()
>  . refs: add refs_head_ref()
>  . refs: move submodule slash stripping code to get_submodule_ref_store
>  . refs.c: refactor get_submodule_ref_store(), share common free block
>  . revision.c: --indexed-objects add objects from all worktrees
>  . revision.c: refactor add_index_objects_to_pending()
>  . revision.h: new flag in struct rev_info wrt. worktree-related refs
>
>  "git gc" and friends when multiple worktrees are used off of a
>  single repository did not consider the index and per-worktree refs
>  of other worktrees as the root for reachability traversal, making
>  objects that are in use only in other worktrees to be subject to
>  garbage collection.

I'm back and will try to continue this. Is it discarded because of
lack of progress, or because the problem is already fixed some other
way? A quick "git log --oneline" on important files has not revealed
anything.
-- 
Duy