Re: persistent-https, url insteadof, and `git submodule`

[Elliott Cable]

Hi! Thanks for the responses (I hope reply-all isn't bad mailing-list
etiquette? Feel free to yell at with a direct reply!). For whatever it's
worth, as a random user, here's my thoughts:

On Sat, May 20, 2017 at 2:07 AM, Jeff King <p...@peff.net> wrote:
> On Fri, May 19, 2017 at 11:55:34PM +0200, Dennis Kaarsemaker wrote:
>> > On Fri, 2017-05-19 at 14:57 -0500, Elliott Cable wrote:
>> > > Presumably this isn't intended behaviour?
>> >
>> > It actually is. git-submodule sets GIT_PROTOCOL_FROM_USER to 0, which
>> > makes git not trust any urls except http(s), git, ssh and file urls
>> > unless you explicitely configure git to allow it. See the
>> > GIT_ALLOW_PROTOCOL section in man git and the git-config section it
>> > links to.
>>
>> 33cfccbbf3 (submodule: allow only certain protocols for submodule
>> fetches, 2015-09-16) says:
>> [...]
>> But doing it this way is
>> simpler, and makes it much less likely that we would miss a
>> case. And since such protocols should be an exception
>> (especially because nobody who clones from them will be able
>> to update the submodules!), it's not likely to inconvenience
>> anyone in practice.
>
> The other approach is to declare that a url rewrite resets the
> protocol-from-user flag to 1. IOW, since the "persistent-https" protocol
> comes from our local config, it's not dangerous and we should behave as
> if the user themselves gave it to us. That makes Elliott's case work out
> of the box.

Well, now that I'm aware of security concerns, `GIT_PROTOCOL_FROM_USER`
and `GIT_ALLOW_PROTOCOL`, and so on, I wouldn't *at all* expect
`insteadOf` to disable that behaviour. Instead, one of two things seems
like a more ideal solution:

1. Most simply, better documentation: mention `GIT_PROTOCOL_FROM_USER`
   explicitly in the documentation of/near `insteadOf`, most
   particularly in the README for `contrib/persistent-https`.

2. Possibly, special-case “higher-security” porcelain (like
   `git-submodule`, as described in 33cfccbbf3) to ignore `insteadOf`
   rewrite-rules without additional, special configuration. This way,
   `git-submodule` works for ignorant users (like me) out of the box,
   just as it previously did, and there's no possible security
   compramise.

Just my 2¢ — thanks for your tireless contributions, loves. <3


⁓ ELLIOTTCABLE — fly safe.
  http://ell.io/tt

persistent-https, url insteadof, and `git submodule`

[Elliott Cable]

Set up `persistent-https` as described in the [README][]; including the
‘rewrite https urls’ feature in `.gitconfig`:

[url "persistent-https"]
insteadof = https
[url "persistent-http"]
insteadof = http

Unfortunately, this breaks `git submodule add`:

> git submodule add https://github.com/nodenv/nodenv.git \
./Vendor/nodenv
Cloning into '/Users/ec/Library/System Repo/Vendor/nodenv'...
fatal: transport 'persistent-https' not allowed
fatal: clone of 'https://github.com/nodenv/nodenv.git' into
submodule path '/Users/ec/Library/System Repo/Vendor/nodenv' failed

Presumably this isn't intended behaviour?

   [README]: 



⁓ ELLIOTTCABLE — fly safe.
  http://ell.io/tt

Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE

[Elliott Cable]

oh, wow, this got over my head *real* fast. Okay,

1. Yeah, my `$GIT_WORK_TREE` was def. an absolute path; I typed that
example code without running it *precisely* that way (entirely my
mistake! I'm so sorry for the confusion it caused, and all that typing
you did!); if I remember correctly (not at the machine right now), I
had run `git rev-parse --show-toplevel` from a different directory,
with `$GIT_DIR` set, while trying to narrow down this bug, so it gave
me an absolute path … and then copy-pasted that path, and then
replaced my copy-paste with the original command to make the
bug-report example as concise as possible? oops. But, yeah, it fails
in the manner described above with an absolute path. (Which it looks
like you two figured out above.)

2. Re: intentions, again, it seems like you've changed your mind, but …

   >  So it is a misconfiguration if you only set GIT_WORK_TREE
without setting GIT_DIR.

   I really, really hope not! Half the usefulness of `$GIT_WORK_TREE`
existing is in that mode. In fact, that's how I found `$GIT_WORK_TREE`
documented and explained, in [this blog
post](https://git-scm.com/blog/2010/04/11/environment.html) on the Git
site. That usage seems pretty damn useful, so I do hope it's
eventually explicitly supported … (and if that's *not* going to be the
case, it should be explicitly documented in the `GIT(1)` manpage,
alongside the other documentation of the environment-variables, that
the behaviour is undefined is `$GIT_DIR` isn't set first. =)


⁓ ELLIOTTCABLE — fly safe.
  http://ell.io/tt
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE

[Elliott Cable]

On Tue, Mar 29, 2016 at 6:42 AM, Elliott Cable <m...@ell.io> wrote:
> So, I find this behaviour a little strange; I can't determine if it's
> a subtle bug, or intentionally undefined/‘fuzzy’ behaviour ...

Oh lord, it gets worse ...

$ cd a-repo
$ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir
true
false
$ cd .git
$ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir
false
true
$ export GIT_WORK_TREE="$(git rev-parse --show-toplevel)"   # !!!
$ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir
true
false
$ # !!?!?

So, basically, if `$GIT_WORK_TREE` is set at all, it appears that the
`rev-parse --is-inside...` flags don't function reliably at all.


⁓ ELLIOTTCABLE — fly safe.
  http://ell.io/tt
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

`git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE

[Elliott Cable]

So, I find this behaviour a little strange; I can't determine if it's
a subtle bug, or intentionally undefined/‘fuzzy’ behaviour:

$ cd a-repo/.git/
$ pwd
/path/to/a-repo/.git
$ git rev-parse --is-inside-work-tree
false
$ export GIT_WORK_TREE=/path/to/a-repo
$ git rev-parse --is-inside-work-tree
true

i.e. when within the repository (the `.git` directory), and when that
directory is a sub-directory of the working-tree, `rev-parse
--is-inside-work-tree` reports *false* (reasonable enough, I suppose);
but then if `$GIT_WORK_TREE` is set to precisely the directory that
git was *already* assuming was the working-directory, then the same
command, in the same location, reports *true*.

This should probably be made consistent: either `rev-parse
--is-inside-work-tree` should report “true”, even inside the `.git`
dir, as long as that directory is a sub-directory of the working-tree
… or repository-directories / `$GIT_DIR` / `.git` directories should
be excluded from truthy responses to `rev-parse
--is-inside-work-tree`.


⁓ ELLIOTTCABLE — fly safe.
  http://ell.io/tt
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

`git rev-parse --git-dir` relative to current working directory?

[Elliott Cable]

So, `git help rev-parse` [mentions the following][rev-parse], as of
2.8.0:

--git-dir
   Show $GIT_DIR if defined. Otherwise show the path to the .git
   directory. The path shown, when relative, is relative to the
   current working directory.

However, when inside a symlinked repository, this doesn't function as
advertised:

$ ln -s a-symlink a-git-repo
$ cd a-symlink/.git/hooks
$ git rev-parse --git-dir
/Users/ec/Documents/a-git-repo/.git

>From my reading of that snippet of documentation (“The path shown ... is
relative to the CWD”), I'd expect to receive `..`, not
`/absolute/path/to/a-git-repo/.git`.

Is the documentation incorrect, or is this a bug? (I'm hoping the
latter: I'm trying to write git-scripting that is sensitive to symlinks,
i.e. retains the user's CWD without unintentionally resolving symlinks
in their path during operation; and it'd be ideal if this were handled
as documented, saving me manual effort checking symlinks.)


⁓ ELLIOTTCABLE — fly safe.
  http://ell.io/tt

   [rev-parse]:

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

`format:%>` padding and `git log --graph`

[Elliott Cable]

I'm not sure what version the `%>` / `<|` / etc padding showed up in,
but they're truly excellent for building beautiful one-line `git log`
output.

This may be a long-shot, but, unfortunately, these new formats sort of
fall flat in the presence of `git log --graph`: The ‘pad until column’
feature, which when reading the manpage, I desperately hoped
*specifically exists* to replace the normal ‘pad with spaces’ in
situations where `--graph` adds an un-known number of characters to the
start of the line ... unfortunately doesn't seem to work that way.

For instance, here's some truncated output from a basic `--graph`:

$ git log --graph --abbrev=8 --pretty="tformat:%h %s"
...
* | a4402023 + basic boilerplate for Liability / LiabilityFamily
* |   32ed6de8 Merge branch 'queueless' into queueless+
|\ \
| * \   1e53ea10 (merge misc) Bring in some `bats` fixes, and re-sty
| |\ \
| | |/
| | * c8c270ff (!! new doc) Add rationale for basically *all* of the

Here's what `%>|(16)%h` gives me:

$ git log --graph --abbrev=8 --pretty="tformat:%>|(16)%h %s"
...
* | a4402023 + basic boilerplate for Liability / LiabilityFa
* |   32ed6de8 Merge branch 'queueless' into queueless+
|\ \
| * \   1e53ea10 (merge misc) Bring in some `bats` fixes, an
| |\ \
| | |/
| | * c8c270ff (!! new doc) Add rationale for basically *all

Here's something like what I'd *like* to have seen:

$ git log --graph --abbrev=8 --pretty="tformat:%>|(16)%h %s"
...
* | a4402023 + basic boilerplate for Liability / LiabilityFamily
* | 32ed6de8 Merge branch 'queueless' into queueless+
|\ \
| * \   1e53ea10 (merge misc) Bring in some `bats` fixes, and re-sty
| * \   1e53ea10 (merge misc) Bring in some `bats` fixes, and re-sty
| |\ \
| | |/
| | *   c8c270ff (!! new doc) Add rationale for basically *all* of t

So: Is this nigh-unimplementable? I once [dove into the git-log
source][patch], and I recall the `--graph` code being terrifying; so if
this is difficult to support, I can see why it would be left out.

If I'm off, though, and this is just an oversight, it'd be really neat
to see somebody implement it! (=


⁓ ELLIOTTCABLE — fly safe.
  http://ell.io/tt

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

Strange situation with --assume-unchanged and diff --find-copies-harder

[Elliott Cable]

So, I've spent some time in the #git channel on Freenode chatting
about this, and we couldn't figure it out. I can't reproduce it in a
newly-made repository, but it's reproducible with the repository I've
been working in.

 git status
On branch Master
Your branch is ahead of 'ec/Master' by 2 commits.
 (use git push to publish your local commits)

nothing to commit, working directory clean
 g diff --find-copies-harder
diff --git i/Executables/paws.js w/Executables/paws.js
old mode 100755
new mode 100644
 stat -f '%p' Executables/paws.js
100755


 - As demonstrated by the `stat`, the mode-change shown by `git diff`
is a phantom change; it never happened.
 - If I remove the `--find-copies-harder` flag, it doesn't show up.
 - If I choose to --no-assume-unchanged the executable, it doesn't show up.
 - If I change the actual file-mode to the 644 it thinks it is, and
commit it, it doesn't show up.

It's only the precise combination of A) a file flagged +x, B) that
file --assume-unchange'd on the index, and C) diff called with the
--find-copies-harder flag, that shows the phantom mode-change.

I tried reproducing that situation in a clean repository, and the
problem didn't seem to surface. You're welcome to clone the repository
in question, however, and reproduce it yourself:

 git clone https://github.com/ELLIOTTCABLE/Paws.js.git
 git update-index --assume-unchanged Executables/paws.js
 git diff --find-copies-harder
 stat -f '%p' Executables/paws.js

I'm working on git 1.9.2.

⁓ ELLIOTTCABLE — fly safe.
  http://ell.io/tt
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC] add --authorship-order flag to git log / rev-list

[Elliott Cable]

On Tue, Jun 4, 2013 at 2:53 PM, Junio C Hamano gits...@pobox.com wrote:
 After reading the subject alone, my reaction was is this sorting
 commits by the name of the author?

 That is one of the expected natural reactions when people hear about
 this option, which is not what you want.

 Perhaps naming it --authordate-order (or enhance the command line
 parsing to allow --date-order=author|committer) would give us a
 better UI.

The same comment was raised by someone in IRC when I submitted an RFC
on this. The conclusion we'd arrived at, IIRC, was that the only
remotely-not-ugly solutions were either --authorship-order or
--author-date-order.

I really like the idea of [--date-order[=author|committer]], but
that's getting beyond my knowledge of the code-base. Perhaps I should
just implement the changes to the implementation in *my* revision of
the patch, and leave it up to a future patcher with the requisite
knowledge of the argumentation features to throw in the changes to
that flag quickly? Either that, or implement it as --author-date-order
right *now*, and change it later before it hits Master (so we don't
end up with a no-longer-supported feature?)

(It'd take me many hours to track down the details of how git's
codebase goes around doing that, and then attempting to replicate it,
whereas someone familiar could probably do it in fifteen minutes,
hence the thought-process. Commentary welcome.)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC] rev-list: add --authorship-order alternative ordering

[Elliott Cable]

On Tue, Jun 4, 2013 at 3:14 PM, Junio C Hamano gits...@pobox.com wrote:
 elliottcable m...@ell.io writes:
 Thus, I've added an --authorship-order version of --date-order, which relies
 upon the AUTHOR_DATE instead of the COMMITTER_DATE; this means that old 
 commits
 will continue to show up chronologically in-order despite rebasing.
 ---

 Missing sign-off.  Please see Documentation/SubmittingPatches.

Will-do.

I read that part, and was rather confused. At no point did I get the
idea that I should sign-off *my own initial commit*. Perhaps that part
of the documentation needs to be slightly re-written? Would that be a
welcome change?

On Tue, Jun 4, 2013 at 3:14 PM, Junio C Hamano gits...@pobox.com wrote:
 elliottcable m...@ell.io writes:
 diff --git a/builtin/log.c b/builtin/log.c
 index 9e21232..54d4d7f 100644
 --- a/builtin/log.c
 +++ b/builtin/log.c
 @@ -237,7 +237,7 @@ static void log_show_early(struct rev_info *revs, struct 
 commit_list *list)
   int i = revs-early_output;
   int show_header = 1;

 - sort_in_topological_order(list, revs-lifo);
 + sort_in_topological_order(list, revs-lifo, revs-use_author);

 The name use-author is a clear sign that the person who added this
 code were too narrowly focused to think author automatically would
 mean author date ;-).

 It probably makes sense to revamp sort_in_topological_order(), so
 that its second parameter is not a boolean 'lifo' that tells too
 much about its implementation without telling what it actually
 means.  Instead, we can make it an enum sort_order, that tells it to
 emit the commits in committer-date order, author-date order, or
 graph-traversal order.

 And update revs-lifo to use that same enum, without adding
 use_author_date bit to rev_info.

I'll look into replacing lifo with an enum as soon as I can sit back
down to update this patch. For the moment, nothing more than
committer_date_sort and author_date_sort, I suppose?

Overview being, I suppose, that `lifo` will no longer exist (since it
effectively determines, when truthy, that we operate in a
*non*-date-ordered topological method); then have commiter_date_order
and author_date_order bits in an enum, with zero being
lifo/straightforward-topological-order. Sound about right?

I'll try and make this a separate patch. First commit, to replace lifo
with an enum; second commit, to *actually implement* the code obeying
that enum when it is set to author_date_order.

On Tue, Jun 4, 2013 at 5:22 PM, Jeff King p...@peff.net wrote:
 On Tue, Jun 04, 2013 at 12:14:21PM -0700, Junio C Hamano wrote:

  diff --git a/commit.h b/commit.h
  index 67bd509..de07525 100644
  --- a/commit.h
  +++ b/commit.h
  @@ -17,6 +17,7 @@ struct commit {
  void *util;
  unsigned int indegree;
  unsigned long date;
  +   unsigned long author_date;

 While walking we keep many of them in-core, and 8-byte each for each
 commit objects add up.  We do not want to make struct commit any
 larger than it already is.

 Having said that, I do not see a reasonable alternative
 implementation than adding an author-date field to struct commit
 without major restructuring if we were to add this feature.

 So please do not take this part of the response as a patch rejected
 because we do not want to add anything to this structure.  We'll
 think of something down the road, but as an independent topic after
 this gets in (or doesn't).

 Yeah, I had the same thought. Maybe this is a good candidate to build on
 top of the jk/commit-info slab experiment. The topo-sort could allocate
 an extra slab for author-date (or even expand the indegree slab to hold
 both indegree and author date), use it during the sort, and then free it
 afterwards.

 Elliott: you can see the relevant changes to the topo-sort in commit
 96c4f4a (commit: allow associating auxiliary info on-demand,
 2013-04-09).

 -Peff

Again, might be a little over my head. If you really think it's best
that I look into that branch, I will try. :)

Meantime, is there any other, more-immediate approach you can think
of? I thought, for a moment, of only storing *either* the committer
*or* the author date in the commit-struct at a given time, and
flagging with a single bit ... but I'm not sure how widely-spread the
nead for committer-date currently is. Maybe I can go back and
parse-out the author date *when I need it*, instead, though that
sounds slow …

Epilogue: I'll make the *obvious* changes mentioned above sometime
within the next week (I'm more than a little swamped; in the middle of
a big breakup, and a big move, simultaneously! :( ), especially the
enum instead of the use_author bit … and submit another patch RFC. It
won't be finalized until we can decide what to do about the extra
8bytes in the commit struct, though. More input welcome.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC] rev-list: add --authorship-order alternative ordering

[Elliott Cable]

Wow. That's my bad entirely. I apparently hallucinated a section
suggesting that you “sign-off” commits that you'd reviewed, or
something; and I'd completely skipped the section on certifying that
you have legal rights to the work, because I'd *written* it, and
didn't think it'd be relevant.

I feel like an idiot. Forgive me. I'll --signoff my next version of
the patch. o7

On Thu, Jun 6, 2013 at 3:29 PM, Junio C Hamano gits...@pobox.com wrote:
 Elliott Cable m...@ell.io writes:

 On Tue, Jun 4, 2013 at 3:14 PM, Junio C Hamano gits...@pobox.com wrote:
 elliottcable m...@ell.io writes:
 Thus, I've added an --authorship-order version of --date-order, which 
 relies
 upon the AUTHOR_DATE instead of the COMMITTER_DATE; this means that old 
 commits
 will continue to show up chronologically in-order despite rebasing.
 ---

 Missing sign-off.  Please see Documentation/SubmittingPatches.

 Will-do.

 I read that part, and was rather confused. At no point did I get the
 idea that I should sign-off *my own initial commit*. Perhaps that part
 of the documentation needs to be slightly re-written? Would that be a
 welcome change?

 I fail to see what more needs to be clarified on top of what we
 already have; please re-read (5) Sign your work section, paying
 with special attention to:

  - YOU WROTE IT or otherwise have the right to pass it on.

  - the contribution was created in whole or in part BY ME and I
HAVE THE RIGHT TO SUBMIT.

 But perhaps you meant something else by *my own initial commit*???
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Feature-request: Ordering `git log --graph` based on *author's* timestamp

[Elliott Cable]

I use a fairly complex `git-log` command involving `--date-order` to
get an overview of my repository's status; but unfortunately,
`--date-order` seems to use the *committer* date, not the *author*
date. That means that each time I bring my topic branches up to date
by rebasing them onto the current upstream, I lose the helpful
chronological ordering in my `git-log` of relative commits in my topic
branches (that is, each branch becomes a single long line, because all
of its commits got rebased to sequential and nearly-identical
committer timestamps.)

After a bit of activity on [a Stack Overflow question][1], I've
realized that this is basically impossible without 1. dumping `git
log`'s `--graph` feature *entirely*, and writing a very complex custom
system to order and re-graph commits, or 2. getting `git log`
**itself** patched to support an `--author-order` flag or something.

(I've never posted here before, forgive me if this is the wrong way to
go about this … the #git channel on Freenode pointed me here. :)

   [1]: 
http://stackoverflow.com/questions/8576503/how-can-i-make-git-log-order-based-on-authors-timestamp
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html