Git development
 help / color / mirror / Atom feed
* How to investigate further a seemingly 'ghost' commit (after a merge)?
From: fan2linux @ 2018-12-06 14:45 UTC (permalink / raw)
  To: git
In-Reply-To: <2070302649.44051.1544107334387.JavaMail.zimbra@laposte.net>

Hello, 

I am trying to understand how a fix from a bug-correction branch vanished and 
the bug found its way back into the main branch after two merges. 

I am using git version 2.19.2. 

Checkouting tag 18.40.1 and checking its graph: 

> $ git checkout 18.40.1 
> 
> $ git log --oneline --graph 
> * ee4721a1 (HEAD, tag: 18.40.1) Merge branch 'suppressionNumAdhCTP' into 'homol2' 
> |\ 
> | * 52aeaf64 Retire le numéro de contrat et le numéro d'adhérent qui étaient affichés en haut de la page et empiétaient sur le préimprimé. 
> |/ 
> * 9f68ec4b (tag: 18.40.0) Merge branch 'cherry-pick-7c956b5f' into 'homol2' 

For the record, here is the git log excerpt on the file in question: 

> $ git log app/CTPCB-AC001/CTPCB-AC001.tex 
> commit 52aeaf64c808c1e3ee8c5cbf5f0221d4e8a7699d 
> Author: Guillaume Ballin < guillaume.ballin@mnt.fr > 
> Date: Tue Nov 13 11:52:03 2018 +0100 
> 
> Retire le numéro de contrat et le numéro d'adhérent qui étaient affichés en haut de la page et empiétaient sur le préimprimé. 
> 
> commit 9336db5c25bb0f3af19183fe1db2fb05ce28b9f3 
> Author: arnavander < arnavander@mnt.fr > 
> Date: Mon Nov 5 14:46:27 2018 +0000 
> 
> Correction retour anomalie de Carte TP 
> 
> Signed-off-by: arnavander < arnavander@mnt.fr > 
> 
> 
> (cherry picked from commit 7c956b5f29bf23c624684c9300a13abecfb451c5) 

Checkouting commit 25ca67a9 and checking its graph: 

> $ git checkout 25ca67a9 
> 
> $ git log --oneline --graph 
> * 25ca67a9 (HEAD) Merge branch 'homol2' into 'homol' 
> |\ 
> | * ee4721a1 (tag: 18.40.1) Merge branch 'suppressionNumAdhCTP' into 'homol2' 
> | |\ 
> | | * 52aeaf64 Retire le numéro de contrat et le numéro d'adhérent qui étaient affichés en haut de la page et empiétaient sur le préimprimé. 
> | |/ 
> | * 9f68ec4b (tag: 18.40.0) Merge branch 'cherry-pick-7c956b5f' into 'homol2' 

Checking the log of that file again: 

> $ git log app/CTPCB-AC001/CTPCB-AC001.tex 
> commit 7c956b5f29bf23c624684c9300a13abecfb451c5 
> Author: arnavander < arnavander@mnt.fr > 
> Date: Mon Nov 5 15:46:27 2018 +0100 
> 
> Correction retour anomalie de Carte TP 
> 
> Signed-off-by: arnavander < arnavander@mnt.fr > 

Commit 52aeaf64c808c1e3ee8c5cbf5f0221d4e8a7699d does not show up any more in 
the pointed to by commit 25ca67a9! 

The fix only deleted a block of lines: 

> $ git diff 9f68ec4b 52aeaf64 
> diff --git a/app/CTPCB-AC001/CTPCB-AC001.tex b/app/CTPCB-AC001/CTPCB-AC001.tex 
> index 148b3ed2..741d3dff 100644 
> --- a/app/CTPCB-AC001/CTPCB-AC001.tex 
> +++ b/app/CTPCB-AC001/CTPCB-AC001.tex 
> @@ -122,12 +122,6 @@ 
> } 
> 
> \newcommand{\CarteTP}{ 
> - \begin{textblock*}{200pt}(50pt,225pt)% 
> - \No contrat : \NUMADH\\ 
> - \ifdefined\NUMABA 
> - \No adhérent : \NUMABA\\ 
> - \fi 
> - \end{textblock*}% 
> \edToAddrList{DADDR} 
> 
> %PREMIERE CARTE 

How a commit can disapear like that from the log of a file and of course not 
patching the file while staying visible in the git log of the branch? 
It's just like if grandchildren forgetted grandparent commit while staying connected 
to parents... 

--
Fan2linux - G. Ballin

^ permalink raw reply

* A case where diff.colorMoved=plain is more sensible than diff.colorMoved=zebra & others
From: Ævar Arnfjörð Bjarmason @ 2018-12-06 13:54 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git

Let's ignore how bad this patch is for git.git, and just focus on how
diff.colorMoved treats it:

    diff --git a/builtin/add.c b/builtin/add.c
    index f65c172299..d1155322ef 100644
    --- a/builtin/add.c
    +++ b/builtin/add.c
    @@ -6,5 +6,3 @@
     #include "cache.h"
    -#include "config.h"
     #include "builtin.h"
    -#include "lockfile.h"
     #include "dir.h"
    diff --git a/builtin/am.c b/builtin/am.c
    index 8f27f3375b..eded15aa8a 100644
    --- a/builtin/am.c
    +++ b/builtin/am.c
    @@ -6,3 +6,2 @@
     #include "cache.h"
    -#include "config.h"
     #include "builtin.h"
    diff --git a/builtin/blame.c b/builtin/blame.c
    index 06a7163ffe..44a754f190 100644
    --- a/builtin/blame.c
    +++ b/builtin/blame.c
    @@ -8,3 +8,2 @@
     #include "cache.h"
    -#include "config.h"
     #include "color.h"
    diff --git a/cache.h b/cache.h
    index ca36b44ee0..ea8d60b94a 100644
    --- a/cache.h
    +++ b/cache.h
    @@ -4,2 +4,4 @@
     #include "git-compat-util.h"
    +#include "config.h"
    +#include "new.h"
     #include "strbuf.h"

This is a common thing that's useful to have highlighted, e.g. we move
includes of config.h to some common file, so I want to se all the
deleted config.h lines as moved into the cache.h line, and then the
"lockfile.h" I removed while I was at it plain remove, and the new
"new.h" plain added.

Exactly that is what you get with diff.colorMoved=plain, but the default
of diff.colorMoved=zebra gets confused by this and highlights no moves
at all, same or "blocks" and "dimmed-zebra".

So at first I thought this had something to do with the many->one
detection, but it seems to be simpler, we just don't detect a move of
1-line with anything but plain, e.g. this works as expected in all modes
and detects the many->one:

    diff --git a/builtin/add.c b/builtin/add.c
    index f65c172299..f4fda75890 100644
    --- a/builtin/add.c
    +++ b/builtin/add.c
    @@ -5,4 +5,2 @@
      */
    -#include "cache.h"
    -#include "config.h"
     #include "builtin.h"
    diff --git a/builtin/branch.c b/builtin/branch.c
    index 0c55f7f065..52e39924d3 100644
    --- a/builtin/branch.c
    +++ b/builtin/branch.c
    @@ -7,4 +7,2 @@

    -#include "cache.h"
    -#include "config.h"
     #include "color.h"
    diff --git a/cache.h b/cache.h
    index ca36b44ee0..d4146dbf8a 100644
    --- a/cache.h
    +++ b/cache.h
    @@ -3,2 +3,4 @@

    +#include "cache.h"
    +#include "config.h"
     #include "git-compat-util.h"

So is there some "must be at least two consecutive lines" condition for
not-plain, or is something else going on here?

^ permalink raw reply

* Re: [PATCH 2/2] commit-graph: fix buffer read-overflow
From: Derrick Stolee @ 2018-12-06 13:11 UTC (permalink / raw)
  To: Josh Steadmon, git
In-Reply-To: <ad2e761f4438ac80e947be0f6831fb6467eb4396.1544048946.git.steadmon@google.com>

On 12/5/2018 5:32 PM, Josh Steadmon wrote:
>   
> +		if (chunk_lookup + GRAPH_CHUNKLOOKUP_WIDTH > data + graph_size) {
> +			error(_("chunk lookup table entry missing; graph file may be incomplete"));
> +			free(graph);
> +			return NULL;
> +		}

Something I forgot earlier: there are several tests in 
t5318-commit-graph.sh that use 'git commit-graph verify' to ensure we 
hit these error conditions on a corrupted commit-graph file. Could you 
try adding a test there that looks for this error message?

Thanks,
-Stolee

^ permalink raw reply

* [PATCH] docs: fix $strict_export text in gitweb.conf.txt
From: Denis Ovsienko @ 2018-12-06 13:10 UTC (permalink / raw)
  To: git

The section used to discussed $gitweb_export_ok and $gitweb_list, but
gitweb Perl code does not have such variables (this likely hangs over
from GITWEB_EXPORT_OK and GITWEB_LIST respectively). Fix the section to
spell $export_ok and $projects_list like the rest of the document.

Signed-off-by: Denis Ovsienko <denis@ovsienko.info>
---
 Documentation/gitweb.conf.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
index c0a326e38..83b4388c2 100644
--- a/Documentation/gitweb.conf.txt
+++ b/Documentation/gitweb.conf.txt
@@ -207,8 +207,8 @@ subsection on linkgit:gitweb[1] manpage.
 
 $strict_export::
 	Only allow viewing of repositories also shown on the overview page.
-	This for example makes `$gitweb_export_ok` file decide if repository is
-	available and not only if it is shown.  If `$gitweb_list` points to
+	This for example makes `$export_ok` file decide if repository is
+	available and not only if it is shown.  If `$projects_list` points to
 	file with list of project, only those repositories listed would be
 	available for gitweb.  Can be set during building gitweb via
 	`GITWEB_STRICT_EXPORT`.  By default this variable is not set, which
-- 
2.17.1



^ permalink raw reply related

* Re: [PATCH v2] l10n: update German translation
From: phillip @ 2018-12-06 12:00 UTC (permalink / raw)
  To: Ralf Thielow, git; +Cc: Matthias Rüster
In-Reply-To: <20181204065430.31033-1-ralf.thielow@gmail.com>

Hi,

thanks for your great work! Just two remarks:

>   #: midx.c:407
> -#, fuzzy, c-format
> +#, c-format
>   msgid "failed to add packfile '%s'"
> -msgstr "Fehler beim Lesen der Reihenfolgedatei '%s'."
> +msgstr "Fehler beim Hinzufügen von Packdatei'%s'."

A Space is missing: "Fehler beim Hinzufügen von Packdatei '%s'."

>   #: run-command.c:1229
> -#, fuzzy, c-format
> +#, c-format
>   msgid "cannot create async thread: %s"
> -msgstr "kann Thread nicht erzeugen: %s"
> +msgstr "Kann Thread für async nicht erzeugen: %s"

I think we should use "Konnte" here.


Best regards,

Phillip


^ permalink raw reply

* Re: Any way to make git-log to enumerate commits?
From: Konstantin Khomoutov @ 2018-12-06 11:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Konstantin Khomoutov, Konstantin Kharlamov, git
In-Reply-To: <xmqqa7ljbimv.fsf@gitster-ct.c.googlers.com>

On Thu, Dec 06, 2018 at 09:31:36AM +0900, Junio C Hamano wrote:

> >> It would be great if git-log has a formatting option to insert an
> >> index of the current commit since HEAD.
> >> 
> >> It would allow after quitting the git-log to immediately fire up "git
> >> rebase -i HEAD~index" instead of "git rebase -i
> >> go-copy-paste-this-long-number-id".
> >
> > This may have little sense in a general case as the history maintained
> > by Git is a graph, not a single line. Hence your prospective approach
> > would only work for cases like `git log` called with the
> > "--first-parent" command-line option.
> 
> I do not see why the "name each rev relative to HEAD" formatting
> option cannot produce HEAD^2~2 etc.
> 
> It would be similar to "git log | git name-rev --stdin" but I do not
> offhand recall if we had a way to tell name-rev to use only HEAD as
> the anchoring point.

My reading was that the OP explicitly wanted to just glance at a single
integer number and use it right away in a subsequent rebase command.

I mean, use one's own short memory instead of copying and pasting.

The way I decided to format the reference in my sketch script — using
HEAD~<n> — is just a byproduct of the fact I was aware both of the
"gitrevisions" manual page and the fact `git name-rev` exists (though I
regretfully was not aware it's able to process a stream of `git log`).

Hence while getting fancy names for revisions would be technically
correct but less error-prone for retyping from memory ;-)


^ permalink raw reply

* Bug: git add --patch does not honor "diff.noprefix"
From: Christian Weiske @ 2018-12-06 10:34 UTC (permalink / raw)
  To: git

Hi,


When running "git add -p" on git version 2.19.2 and "diff.noprefix" set
to true, it still shows the "a/" and "b/" prefixes.

This issue has been reported in 2016 already[1], but is still there in
2.19.2.


[1]
https://public-inbox.org/git/E1D7329A-A54B-4D09-A72A-62ECA8005752@gmail.com/T/

-- 
Regards/Mit freundlichen Grüßen
Christian Weiske

-=≡ Geeking around in the name of science since 1982 ≡=-

^ permalink raw reply

* [ANNOUNCE] Git Contributor Summit Registration, Jan 31, 2019, Brussels
From: Jeff King @ 2018-12-06  9:48 UTC (permalink / raw)
  To: git

Registration is now open for the Contributor Summit at Git Merge. To
recap from my earlier announcement[1]:

  When: Thursday, January 31, 2019. 10am-5pm.
  Where: The Egg[2], Brussels, Belgium
  What: Round-table discussion about Git
  Who: All contributors to Git or related projects in the Git ecosystem
       are invited; if you're not sure if you qualify, please ask!

Registering for the contributor summit requires a special code. Please
email me off-list to get one.

As with past years, the code will unlock a ticket for both the contrib
summit _and_ the main conference (on Friday, Feb 1st). So don't register
separately for the main conference. Also, as in past years, there are
two variants: a free ticket, and a €99 one. You are free to use either;
if you choose to pay, the money goes entirely to the Software Freedom
Conservancy.

If you'd like to come but need financial assistance with travel costs,
please reach out to the Git PLC at git@sfconservancy.org. And please do
so soon (let's say by Dec 13th), so we can make decisions and people can
book their travel.

-Peff

[1] https://public-inbox.org/git/20181109104202.GA8717@sigill.intra.peff.net/

[2] This is the same venue as 2017: https://goo.gl/maps/E36qCGJhK8J2

^ permalink raw reply

* Re: git, monorepos, and access control
From: Jeff King @ 2018-12-06  9:30 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Junio C Hamano, Coiner, John, git@vger.kernel.org
In-Reply-To: <8736rbypy3.fsf@evledraar.gmail.com>

On Thu, Dec 06, 2018 at 10:17:24AM +0100, Ævar Arnfjörð Bjarmason wrote:

> > The other major user of that feature I can think of is LFS. There Git
> > ends up diffing the LFS pointers, not the big files. Which arguably is
> > the wrong thing (you'd prefer to see the actual file contents diffed),
> > but I think nobody cares in practice because large files generally don't
> > have readable diffs anyway.
> 
> I don't use this either, but I can imagine people who use binary files
> via clean/smudge would be well served by dumping out textual metadata of
> the file for diffing instead of showing nothing.
> 
> E.g. for a video file I might imagine having lines like:
> 
>     duration-seconds: 123
>     camera-model: Shiny Thingamabob
> 
> Then when you check in a new file your "git diff" will show (using
> normal diff view) that:
> 
>    - duration-seconds: 123
>    + duration-seconds: 321
>     camera-model: Shiny Thingamabob

I think that's orthogonal to clean/smudge, though. Neither the in-repo
nor on-disk formats are going to show that kind of output. For that
you'd want a separate textconv filter (and fun fact: showing exif data
was actually the original use case for which I wrote textconv).

If you are using something like LFS, using textconv on top is a little
trickier, because we'd always feed the filter the LFS pointer file, not
the actual data contents. Doing the "reversal" that Junio suggested
would fix that. Or with the code as it is, you can simply define your
filter to convert the LFS pointer data into the real content. I don't
really use LFS, but it looks like:

  [diff "mp4"]
  textconv = git lfs smudge | extract-metadata

would probably work.

-Peff

^ permalink raw reply

* Re: git, monorepos, and access control
From: Ævar Arnfjörð Bjarmason @ 2018-12-06  9:17 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Coiner, John, git@vger.kernel.org
In-Reply-To: <20181206072002.GA29787@sigill.intra.peff.net>


On Thu, Dec 06 2018, Jeff King wrote:

> On Thu, Dec 06, 2018 at 10:08:57AM +0900, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>> > In my opinion this feature is so contrary to Git's general assumptions
>> > that it's likely to create a ton of information leaks of the supposedly
>> > protected data.
>> > ...
>>
>> Yup, with s/implemented/designed/, I agree all you said here
>> (snipped).
>
> Heh, yeah, I actually scratched my head over what word to use. I think
> Git _could_ be written in a way that is both compatible with existing
> repositories (i.e., is still recognizably Git) and is careful about
> object access control. But either way, what we have now is not close to
> that.
>
>> > Sorry I don't have a more positive response. What you want to do is
>> > perfectly reasonable, but I just think it's a mismatch with how Git
>> > works (and because of the security impact, one missed corner case
>> > renders the whole thing useless).
>>
>> Yup, again.
>>
>> Storing source files encrypted and decrypting with smudge filter
>> upon checkout (and those without the access won't get keys and will
>> likely to use sparse checkout to exclude these priviledged sources)
>> is probably the only workaround that does not involve submodules.
>> Viewing "diff" and "log -p" would still be a challenge, which
>> probably could use the same filter as smudge for textconv.
>
> I suspect there are going to be some funny corner cases there. I use:
>
>   [diff "gpg"]
>   textconv = gpg -qd --no-tty
>
> which works pretty well, but it's for files which are _never_ decrypted
> by Git. So they're encrypted in the working tree too, and I don't use
> clean/smudge filters.
>
> If the files are already decrypted in the working tree, then running
> them through gpg again would be the wrong thing. I guess for a diff
> against the working tree, we would always do a "clean" operation to
> produce the encrypted text, and then decrypt the result using textconv.
> Which would work, but is rather slow.
>
>> I wonder (and this is the primary reason why I am responding to you)
>> if it is common enough wish to use the same filter for smudge and
>> textconv?  So far, our stance (which can be judged from the way the
>> clean/smudge filters are named) has been that the in-repo
>> representation is the canonical, and the representation used in the
>> checkout is ephemeral, and that is why we run "diff", "grep",
>> etc. over the in-repo representation, but the "encrypted in repo,
>> decrypted in checkout" abuse would be helped by an option to do the
>> reverse---find changes and look substrings in the representation
>> used in the checkout.  I am not sure if there are other use cases
>> that is helped by such an option.
>
> Hmm. Yeah, I agree with your line of reasoning here. I'm not sure how
> common it is. This is the first I can recall it. And personally, I have
> never really used clean/smudge filters myself, beyond some toy
> experiments.
>
> The other major user of that feature I can think of is LFS. There Git
> ends up diffing the LFS pointers, not the big files. Which arguably is
> the wrong thing (you'd prefer to see the actual file contents diffed),
> but I think nobody cares in practice because large files generally don't
> have readable diffs anyway.

I don't use this either, but I can imagine people who use binary files
via clean/smudge would be well served by dumping out textual metadata of
the file for diffing instead of showing nothing.

E.g. for a video file I might imagine having lines like:

    duration-seconds: 123
    camera-model: Shiny Thingamabob

Then when you check in a new file your "git diff" will show (using
normal diff view) that:

   - duration-seconds: 123
   + duration-seconds: 321
    camera-model: Shiny Thingamabob

etc.

^ permalink raw reply

* Git fetch prints unchanged branches‏
From: stav alfi @ 2018-12-06  8:24 UTC (permalink / raw)
  To: git

Hi,

When running `git fetch` it returns every time the same 100+ branches
that didn't change at all but still specifies them as new branches in
the server. It also prints the branches that did change.

I don't see this behavior in other repositories I contribute. how do I fix it?

The same output of `git fetch` multiple times:

    ....

    From github.com:Stratoscale/mui
    + 499a9ae65...63b55f08e Itai/ui-2837-validate-user-name  ->
origin/Itai/ui-2837-validate-user-name  (forced update)
    * [new branch]          aaaa1                            -> origin/aaaa1
    * [new branch]          aaaa2                            -> origin/aaaa2
    * [new branch]          aaaa3                            -> origin/aaaa3
    ...


Thanks

^ permalink raw reply

* Re: git, monorepos, and access control
From: Jeff King @ 2018-12-06  7:23 UTC (permalink / raw)
  To: Coiner, John
  Cc: Ævar Arnfjörð Bjarmason, Duy Nguyen,
	Derrick Stolee, Git Mailing List
In-Reply-To: <cdeb9dc9-ac25-23c8-f3b9-9a7987be7df0@amd.com>

On Wed, Dec 05, 2018 at 11:42:09PM +0000, Coiner, John wrote:

> > For instance, Git is very eager to try to find delta-compression
> > opportunities between objects, even if they don't have any relationship
> > within the tree structure. So imagine I want to know the contents of
> > tree X. I push up a tree Y similar to X, then fetch it back, falsely
> > claiming to have X but not Y. If the server generates a delta, that may
> > reveal information about X (which you can then iterate to send Y', and
> > so on, treating the server as an oracle until you've guessed the content
> > of X).
> Another good point. I wouldn't have thought of either of these attacks. 
> You're scaring me (appropriately) about the risks of adding security to 
> a previously-unsecured interface. Let me push on the smudge/clean 
> approach and maybe that will bear fruit.

If you do look into that approach, check out how git-lfs works. In fact,
you might even be able to build around lfs itself. It's already putting
placeholder objects into the repository, and then faulting them in from
external storage. All you would need to do is lock down access to that
external storage, which is typically accessed via http.

(That all assumes you're OK with sharing the actual filenames with
everybody, and just restricting access to the blob contents. There's no
way to clean/smudge a whole subtree. For that you'd have to use
submodules).

-Peff

^ permalink raw reply

* Re: git, monorepos, and access control
From: Jeff King @ 2018-12-06  7:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Coiner, John, git@vger.kernel.org
In-Reply-To: <xmqqwoona2c6.fsf@gitster-ct.c.googlers.com>

On Thu, Dec 06, 2018 at 10:08:57AM +0900, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > In my opinion this feature is so contrary to Git's general assumptions
> > that it's likely to create a ton of information leaks of the supposedly
> > protected data.
> > ...
> 
> Yup, with s/implemented/designed/, I agree all you said here
> (snipped).

Heh, yeah, I actually scratched my head over what word to use. I think
Git _could_ be written in a way that is both compatible with existing
repositories (i.e., is still recognizably Git) and is careful about
object access control. But either way, what we have now is not close to
that.

> > Sorry I don't have a more positive response. What you want to do is
> > perfectly reasonable, but I just think it's a mismatch with how Git
> > works (and because of the security impact, one missed corner case
> > renders the whole thing useless).
> 
> Yup, again.
> 
> Storing source files encrypted and decrypting with smudge filter
> upon checkout (and those without the access won't get keys and will
> likely to use sparse checkout to exclude these priviledged sources)
> is probably the only workaround that does not involve submodules.
> Viewing "diff" and "log -p" would still be a challenge, which
> probably could use the same filter as smudge for textconv.

I suspect there are going to be some funny corner cases there. I use:

  [diff "gpg"]
  textconv = gpg -qd --no-tty

which works pretty well, but it's for files which are _never_ decrypted
by Git. So they're encrypted in the working tree too, and I don't use
clean/smudge filters.

If the files are already decrypted in the working tree, then running
them through gpg again would be the wrong thing. I guess for a diff
against the working tree, we would always do a "clean" operation to
produce the encrypted text, and then decrypt the result using textconv.
Which would work, but is rather slow.

> I wonder (and this is the primary reason why I am responding to you)
> if it is common enough wish to use the same filter for smudge and
> textconv?  So far, our stance (which can be judged from the way the
> clean/smudge filters are named) has been that the in-repo
> representation is the canonical, and the representation used in the
> checkout is ephemeral, and that is why we run "diff", "grep",
> etc. over the in-repo representation, but the "encrypted in repo,
> decrypted in checkout" abuse would be helped by an option to do the
> reverse---find changes and look substrings in the representation
> used in the checkout.  I am not sure if there are other use cases
> that is helped by such an option.

Hmm. Yeah, I agree with your line of reasoning here. I'm not sure how
common it is. This is the first I can recall it. And personally, I have
never really used clean/smudge filters myself, beyond some toy
experiments.

The other major user of that feature I can think of is LFS. There Git
ends up diffing the LFS pointers, not the big files. Which arguably is
the wrong thing (you'd prefer to see the actual file contents diffed),
but I think nobody cares in practice because large files generally don't
have readable diffs anyway.

-Peff

^ permalink raw reply

* Re: [RFC PATCH 3/3] test-lib: add the '--stress' option to run a test repeatedly under load
From: Junio C Hamano @ 2018-12-06  6:41 UTC (permalink / raw)
  To: Jeff King; +Cc: SZEDER Gábor, git
In-Reply-To: <20181206053525.GA29481@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Each "wait" will try to collect all processes, but may be interrupted by
> a signal. So the correct number is actually "1 plus the number of times
> the user hits ^C".

Yeah and that is not bounded.  It is OK not to catch multiple ^C
that races with what we do, so having ane extra wait in the clean-up
procedure after receiving a signal like you suggested would be both
good enough and the cleanest solution, I think.

Thanks.

^ permalink raw reply

* Re: [WIP RFC 4/5] upload-pack: refactor writing of "packfile" line
From: Junio C Hamano @ 2018-12-06  6:35 UTC (permalink / raw)
  To: Jonathan Tan; +Cc: git
In-Reply-To: <1d678a78a63b7e58988b52c8c0efab38c34a6848.1543879256.git.jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

> @@ -126,6 +129,12 @@ static int read_pack_objects_stdout(int outfd, struct output_state *os)
>  	}
>  	os->used += readsz;
>  
> +	if (!os->packfile_started) {
> +		os->packfile_started = 1;
> +		if (use_protocol_v2)
> +			packet_write_fmt(1, "packfile\n");

If we fix this function so that the only byte in the buffer is held
back without emitted when os->used == 1 as I alluded to, this may
have to be done a bit later, as with such a change, it is no longer
guaranteed that send_client_data() will be called after this point.

> +	}
> +
>  	if (os->used > 1) {
>  		send_client_data(1, os->buffer, os->used - 1);
>  		os->buffer[0] = os->buffer[os->used - 1];

> +static void flush_progress_buf(struct strbuf *progress_buf)
> +{
> +	if (!progress_buf->len)
> +		return;
> +	send_client_data(2, progress_buf->buf, progress_buf->len);
> +	strbuf_reset(progress_buf);
> +}

Interesting.

>  static void create_pack_file(const struct object_array *have_obj,
> -			     const struct object_array *want_obj)
> +			     const struct object_array *want_obj,
> +			     int use_protocol_v2)
>  {
>  	struct child_process pack_objects = CHILD_PROCESS_INIT;
>  	struct output_state output_state = {0};
> @@ -260,9 +278,13 @@ static void create_pack_file(const struct object_array *have_obj,
>  			 */
>  			sz = xread(pack_objects.err, progress,
>  				  sizeof(progress));
> -			if (0 < sz)
> -				send_client_data(2, progress, sz);
> -			else if (sz == 0) {
> +			if (0 < sz) {
> +				if (output_state.packfile_started)
> +					send_client_data(2, progress, sz);
> +				else
> +					strbuf_add(&output_state.progress_buf,
> +						   progress, sz);

Isn't progress output that goes to the channel #2 pretty much
independent from the payload stream that carries the pkt-line 
command like "packfile" plus the raw pack stream?  It somehow
feels like an oxymoron to _buffer_ progress indicator, as it
defeats the whole point of progress report to buffer it.

^ permalink raw reply

* Re: [RFC PATCH 3/3] test-lib: add the '--stress' option to run a test repeatedly under load
From: Jeff King @ 2018-12-06  5:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: SZEDER Gábor, git
In-Reply-To: <xmqqefavbj28.fsf@gitster-ct.c.googlers.com>

On Thu, Dec 06, 2018 at 09:22:23AM +0900, Junio C Hamano wrote:

> > So the right number of waits is either "1" or "2". Looping means we do
> > too many (which is mostly a harmless noop) or too few (in the off chance
> > that you have only a single job ;) ). So it works out in practice.
> 
> Well, if you time your ^C perfectly, no number of waits is right, I
> am afraid.  You spawn N processes and while looping N times waiting
> for them, you can ^C out of wait before these N processes all die,
> no?

Each "wait" will try to collect all processes, but may be interrupted by
a signal. So the correct number is actually "1 plus the number of times
the user hits ^C". I had assumed the user was just hitting it once,
though putting the wait into the trap means we do that "1 plus" thing
anyway.

I could also see an argument that subsequent ^C's should exit
immediately, but I think we are getting well into the realm of
over-engineering.

-Peff

^ permalink raw reply

* Re: [PATCH 1/2] commit-graph, fuzz: Add fuzzer for commit-graph
From: Junio C Hamano @ 2018-12-06  4:47 UTC (permalink / raw)
  To: Josh Steadmon; +Cc: git, stolee
In-Reply-To: <53e62baaa8769bf8e90991e32e0d123cc6629559.1544048946.git.steadmon@google.com>

Josh Steadmon <steadmon@google.com> writes:

> diff --git a/fuzz-commit-graph.c b/fuzz-commit-graph.c
> new file mode 100644
> index 0000000000..420851d0d2
> --- /dev/null
> +++ b/fuzz-commit-graph.c
> @@ -0,0 +1,18 @@
> +#include "object-store.h"
> +#include "commit-graph.h"
> +
> +struct commit_graph *parse_commit_graph(void *graph_map, int fd,
> +					size_t graph_size);
> +
> +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
> +
> +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
> +{
> +	struct commit_graph *g;
> +
> +	g = parse_commit_graph((void *) data, -1, size);
> +	if (g)
> +		free(g);

As it is perfectly OK to free(NULL), please lose "if (g)" and a
level of indentation; otherwise, "make coccicheck" would complain.

Thanks.

> +	return 0;
> +}

^ permalink raw reply

* Re: [PATCH 1/2] commit-graph, fuzz: Add fuzzer for commit-graph
From: Junio C Hamano @ 2018-12-06  1:41 UTC (permalink / raw)
  To: Josh Steadmon; +Cc: git, stolee
In-Reply-To: <xmqqmupja19b.fsf@gitster-ct.c.googlers.com>

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

>> +	if (graph_size < GRAPH_MIN_SIZE)
>> +		return NULL;
>> +
>
> The load_commit_graph_one() grabbed graph_map out of xmmap() so it
> is guaranteed to be non-NULL, but we need to check graph_map != NULL
> when we're calling this directly from the fuzz tests, exactly in the
> same spirit that we check graph_size above.  Besides, these are to
> make sure future callers won't misuse the API.

Insert "Please check graph_map != NULL here, too." before the above
paragraph.

>>  	data = (const unsigned char *)graph_map;
>
> And the reset of the function is the same as the original modulo
> jumping to the cleanup_fail label has been replaced with returning
> NULL.
>
> Looks good.

Of course, s/reset/rest/ is what I meant.

^ permalink raw reply

* Re: [PATCH 1/2] commit-graph, fuzz: Add fuzzer for commit-graph
From: Junio C Hamano @ 2018-12-06  1:32 UTC (permalink / raw)
  To: Josh Steadmon; +Cc: git, stolee
In-Reply-To: <53e62baaa8769bf8e90991e32e0d123cc6629559.1544048946.git.steadmon@google.com>

Josh Steadmon <steadmon@google.com> writes:

> @@ -108,27 +106,61 @@ struct commit_graph *load_commit_graph_one(const char *graph_file)
>  		die(_("graph file %s is too small"), graph_file);
>  	}
>  	graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0);
> +	ret = parse_commit_graph(graph_map, fd, graph_size);

OK, assuming that the new helper returns NULL from all places in the
original that would have jumped to the cleanup-fail label, this would
do the same thing (it may not be the right thing to exit, but that
is OK for the purpose of this change).

> +	if (ret == NULL) {
> +		munmap(graph_map, graph_size);
> +		close(fd);
> +		exit(1);
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * This function is intended to be used only from load_commit_graph_one() or in
> + * fuzz tests.
> + */

Hmph, is that necessary to say?  

"Right now, it has only these two callers" is sometimes handy for
those without good static analysis tools, like "git grep" ;-), but I
do not think of a reason why a new caller we'll add in the future,
who happens to have the data of the graph file in-core, should not
be allowed to call this function.


> +struct commit_graph *parse_commit_graph(void *graph_map, int fd,
> +					size_t graph_size)
> +{
> +	const unsigned char *data, *chunk_lookup;
> +	uint32_t i;
> +	struct commit_graph *graph;
> +	uint64_t last_chunk_offset;
> +	uint32_t last_chunk_id;
> +	uint32_t graph_signature;
> +	unsigned char graph_version, hash_version;
> +
> +	/*
> +	 * This should already be checked in load_commit_graph_one, but we still
> +	 * need a check here for when we're calling parse_commit_graph directly
> +	 * from fuzz tests. We can omit the error message in that case.
> +	 */

In the same spirit, I am dubious of the longer-term value of this
comment.  As an explanation of why this conversion is correct in the
proposed log message for this change, it perfectly makes sense,
though.

> +	if (graph_size < GRAPH_MIN_SIZE)
> +		return NULL;
> +

The load_commit_graph_one() grabbed graph_map out of xmmap() so it
is guaranteed to be non-NULL, but we need to check graph_map != NULL
when we're calling this directly from the fuzz tests, exactly in the
same spirit that we check graph_size above.  Besides, these are to
make sure future callers won't misuse the API.

>  	data = (const unsigned char *)graph_map;

And the reset of the function is the same as the original modulo
jumping to the cleanup_fail label has been replaced with returning
NULL.

Looks good.

> ...
> -
> -cleanup_fail:
> -	munmap(graph_map, graph_size);
> -	close(fd);
> -	exit(1);
>  }
>  
>  static void prepare_commit_graph_one(struct repository *r, const char *obj_dir)
> diff --git a/fuzz-commit-graph.c b/fuzz-commit-graph.c
> new file mode 100644
> index 0000000000..420851d0d2
> --- /dev/null
> +++ b/fuzz-commit-graph.c
> @@ -0,0 +1,18 @@
> +#include "object-store.h"
> +#include "commit-graph.h"
> +
> +struct commit_graph *parse_commit_graph(void *graph_map, int fd,
> +					size_t graph_size);
> +
> +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
> +
> +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
> +{
> +	struct commit_graph *g;
> +
> +	g = parse_commit_graph((void *) data, -1, size);
> +	if (g)
> +		free(g);
> +
> +	return 0;
> +}

^ permalink raw reply

* Re: [PATCH v3] list-objects.c: don't segfault for missing cmdline objects
From: Junio C Hamano @ 2018-12-06  1:12 UTC (permalink / raw)
  To: Matthew DeVore; +Cc: git, jonathantanmy, jeffhost, ramsay, matvore
In-Reply-To: <20181205214346.106217-1-matvore@google.com>

Matthew DeVore <matvore@google.com> writes:

> When a command is invoked with both --exclude-promisor-objects,
> --objects-edge-aggressive, and a missing object on the command line,
> the rev_info.cmdline array could get a NULL pointer for the value of
> an 'item' field. Prevent dereferencing of a NULL pointer in that
> situation.
>
> Properly handle --ignore-missing. If it is not passed, die when an
> object is missing. Otherwise, just silently ignore it.
>
> Signed-off-by: Matthew DeVore <matvore@google.com>

Thanks for an update.  Will replace.

> ---
>  revision.c               |  2 ++
>  t/t0410-partial-clone.sh | 16 ++++++++++++++--
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/revision.c b/revision.c
> index 13e0519c02..293303b67d 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1729,6 +1729,8 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
>  	if (!cant_be_filename)
>  		verify_non_filename(revs->prefix, arg);
>  	object = get_reference(revs, arg, &oid, flags ^ local_flags);
> +	if (!object)
> +		return revs->ignore_missing ? 0 : -1;
>  	add_rev_cmdline(revs, object, arg_, REV_CMD_REV, flags ^ local_flags);
>  	add_pending_object_with_path(revs, object, arg, oc.mode, oc.path);
>  	free(oc.path);
> diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
> index ba3887f178..169f7f10a7 100755
> --- a/t/t0410-partial-clone.sh
> +++ b/t/t0410-partial-clone.sh
> @@ -349,7 +349,7 @@ test_expect_success 'rev-list stops traversal at promisor commit, tree, and blob
>  	grep $(git -C repo rev-parse bar) out  # sanity check that some walking was done
>  '
>  
> -test_expect_success 'rev-list accepts missing and promised objects on command line' '
> +test_expect_success 'rev-list dies for missing objects on cmd line' '
>  	rm -rf repo &&
>  	test_create_repo repo &&
>  	test_commit -C repo foo &&
> @@ -366,7 +366,19 @@ test_expect_success 'rev-list accepts missing and promised objects on command li
>  
>  	git -C repo config core.repositoryformatversion 1 &&
>  	git -C repo config extensions.partialclone "arbitrary string" &&
> -	git -C repo rev-list --exclude-promisor-objects --objects "$COMMIT" "$TREE" "$BLOB"
> +
> +	for OBJ in "$COMMIT" "$TREE" "$BLOB"; do
> +		test_must_fail git -C repo rev-list --objects \
> +			--exclude-promisor-objects "$OBJ" &&
> +		test_must_fail git -C repo rev-list --objects-edge-aggressive \
> +			--exclude-promisor-objects "$OBJ" &&
> +
> +		# Do not die or crash when --ignore-missing is passed.
> +		git -C repo rev-list --ignore-missing --objects \
> +			--exclude-promisor-objects "$OBJ" &&
> +		git -C repo rev-list --ignore-missing --objects-edge-aggressive \
> +			--exclude-promisor-objects "$OBJ"
> +	done
>  '
>  
>  test_expect_success 'gc repacks promisor objects separately from non-promisor objects' '

^ permalink raw reply

* Re: git, monorepos, and access control
From: Junio C Hamano @ 2018-12-06  1:08 UTC (permalink / raw)
  To: Jeff King; +Cc: Coiner, John, git@vger.kernel.org
In-Reply-To: <20181205210104.GC19936@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> In my opinion this feature is so contrary to Git's general assumptions
> that it's likely to create a ton of information leaks of the supposedly
> protected data.
> ...

Yup, with s/implemented/designed/, I agree all you said here
(snipped).

> Sorry I don't have a more positive response. What you want to do is
> perfectly reasonable, but I just think it's a mismatch with how Git
> works (and because of the security impact, one missed corner case
> renders the whole thing useless).

Yup, again.

Storing source files encrypted and decrypting with smudge filter
upon checkout (and those without the access won't get keys and will
likely to use sparse checkout to exclude these priviledged sources)
is probably the only workaround that does not involve submodules.
Viewing "diff" and "log -p" would still be a challenge, which
probably could use the same filter as smudge for textconv.

I wonder (and this is the primary reason why I am responding to you)
if it is common enough wish to use the same filter for smudge and
textconv?  So far, our stance (which can be judged from the way the
clean/smudge filters are named) has been that the in-repo
representation is the canonical, and the representation used in the
checkout is ephemeral, and that is why we run "diff", "grep",
etc. over the in-repo representation, but the "encrypted in repo,
decrypted in checkout" abuse would be helped by an option to do the
reverse---find changes and look substrings in the representation
used in the checkout.  I am not sure if there are other use cases
that is helped by such an option.

^ permalink raw reply

* Re: [PATCH 1/2] commit-graph, fuzz: Add fuzzer for commit-graph
From: Josh Steadmon @ 2018-12-06  1:00 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, stolee
In-Reply-To: <874lbrzj2d.fsf@evledraar.gmail.com>

On 2018.12.05 23:48, Ævar Arnfjörð Bjarmason wrote:
> 
> On Wed, Dec 05 2018, Josh Steadmon wrote:
> 
> > Breaks load_commit_graph_one() into a new function,
> > parse_commit_graph(). The latter function operates on arbitrary buffers,
> > which makes it suitable as a fuzzing target.
> >
> > Adds fuzz-commit-graph.c, which provides a fuzzing entry point
> > compatible with libFuzzer (and possibly other fuzzing engines).
> >
> > Signed-off-by: Josh Steadmon <steadmon@google.com>
> > ---
> >  .gitignore          |  1 +
> >  Makefile            |  1 +
> >  commit-graph.c      | 63 +++++++++++++++++++++++++++++++++------------
> >  fuzz-commit-graph.c | 18 +++++++++++++
> >  4 files changed, 66 insertions(+), 17 deletions(-)
> >  create mode 100644 fuzz-commit-graph.c
> >
> > diff --git a/.gitignore b/.gitignore
> > index 0d77ea5894..8bcf153ed9 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -1,3 +1,4 @@
> > +/fuzz-commit-graph
> >  /fuzz_corpora
> >  /fuzz-pack-headers
> >  /fuzz-pack-idx
> > diff --git a/Makefile b/Makefile
> > index 1a44c811aa..6b72f37c29 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -684,6 +684,7 @@ SCRIPTS = $(SCRIPT_SH_INS) \
> >
> >  ETAGS_TARGET = TAGS
> >
> > +FUZZ_OBJS += fuzz-commit-graph.o
> >  FUZZ_OBJS += fuzz-pack-headers.o
> >  FUZZ_OBJS += fuzz-pack-idx.o
> >
> > diff --git a/commit-graph.c b/commit-graph.c
> > index 40c855f185..0755359b1a 100644
> > --- a/commit-graph.c
> > +++ b/commit-graph.c
> > @@ -46,6 +46,10 @@
> >  #define GRAPH_MIN_SIZE (GRAPH_HEADER_SIZE + 4 * GRAPH_CHUNKLOOKUP_WIDTH \
> >  			+ GRAPH_FANOUT_SIZE + GRAPH_OID_LEN)
> >
> > +struct commit_graph *parse_commit_graph(void *graph_map, int fd,
> > +					size_t graph_size);
> > +
> > +
> >  char *get_commit_graph_filename(const char *obj_dir)
> >  {
> >  	return xstrfmt("%s/info/commit-graph", obj_dir);
> > @@ -84,16 +88,10 @@ static int commit_graph_compatible(struct repository *r)
> >  struct commit_graph *load_commit_graph_one(const char *graph_file)
> >  {
> >  	void *graph_map;
> > -	const unsigned char *data, *chunk_lookup;
> >  	size_t graph_size;
> >  	struct stat st;
> > -	uint32_t i;
> > -	struct commit_graph *graph;
> > +	struct commit_graph *ret;
> >  	int fd = git_open(graph_file);
> > -	uint64_t last_chunk_offset;
> > -	uint32_t last_chunk_id;
> > -	uint32_t graph_signature;
> > -	unsigned char graph_version, hash_version;
> >
> >  	if (fd < 0)
> >  		return NULL;
> > @@ -108,27 +106,61 @@ struct commit_graph *load_commit_graph_one(const char *graph_file)
> >  		die(_("graph file %s is too small"), graph_file);
> >  	}
> >  	graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0);
> > +	ret = parse_commit_graph(graph_map, fd, graph_size);
> > +
> > +	if (ret == NULL) {
> 
> Code in git usually uses just !ret.

Will fix in V2, thanks.


> > +		munmap(graph_map, graph_size);
> > +		close(fd);
> > +		exit(1);
> 
> Ouch, exit(1) from load_commit_graph_one()? Can't we return NULL here
> instead? Nasty to exit from a library routine, but then I see later...
> 
> > @@ -201,11 +235,6 @@ struct commit_graph *load_commit_graph_one(const char *graph_file)
> >  	}
> >
> >  	return graph;
> > -
> > -cleanup_fail:
> > -	munmap(graph_map, graph_size);
> > -	close(fd);
> > -	exit(1);
> >  }
> 
> ... ah, I see this is where you got the exit(1) from. So it was there
> already.
> 
> >  static void prepare_commit_graph_one(struct repository *r, const char *obj_dir)
> > diff --git a/fuzz-commit-graph.c b/fuzz-commit-graph.c
> > new file mode 100644
> > index 0000000000..420851d0d2
> > --- /dev/null
> > +++ b/fuzz-commit-graph.c
> > @@ -0,0 +1,18 @@
> > +#include "object-store.h"
> > +#include "commit-graph.h"
> > +
> > +struct commit_graph *parse_commit_graph(void *graph_map, int fd,
> > +					size_t graph_size);
> > +
> > +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
> > +
> > +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
> > +{
> > +	struct commit_graph *g;
> > +
> > +	g = parse_commit_graph((void *) data, -1, size);
> > +	if (g)
> > +		free(g);
> > +
> > +	return 0;
> > +}
> 
> I hadn't looked at this before, but see your 5e47215080 ("fuzz: add
> basic fuzz testing target.", 2018-10-12) for some prior art.
> 
> There's instructions there for a very long "make" invocation. Would be
> nice if this were friendlier and we could just do "make test-fuzz" or
> something...

Yeah, the problem is that there are too many combinations of fuzzing
engine, sanitizer, and compiler to make any reasonable default here.
Even if you just stick with libFuzzer, address sanitizer, and clang, the
flags change radically depending on which version of clang you're using.

^ permalink raw reply

* Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF
From: Junio C Hamano @ 2018-12-06  0:58 UTC (permalink / raw)
  To: Frank Schäfer; +Cc: Johannes Sixt, brian m. carlson, git
In-Reply-To: <7796f0ac-d3db-68f9-89fa-9262d2187f57@googlemail.com>

Frank Schäfer <fschaefer.oss@googlemail.com> writes:

> Just to be sure that I'm not missing anything here:
> What's your definition of "LF in repository, CRLF in working tree" in
> terms of config parameters ?

:::Documentation/config/core.txt:::

core.autocrlf::
	Setting this variable to "true" is the same as setting
	the `text` attribute to "auto" on all files and core.eol to "crlf".
	Set to true if you want to have `CRLF` line endings in your
	working directory and the repository has LF line endings.
	This variable can be set to 'input',
	in which case no output conversion is performed.

^ permalink raw reply

* Re: gitweb: local configuration not found
From: Junio C Hamano @ 2018-12-06  0:56 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Jonathan Nieder, Martin Mares, git
In-Reply-To: <87bm5zzt4a.fsf@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>> Documentation says "If you are absolutely certain that you want your
>> script to load and execute a file from the current directory, then use
>> a ./ prefix".  We can do that, like so:
>>
>> diff --git i/gitweb/Makefile w/gitweb/Makefile
>> index cd194d057f..3160b6cc5d 100644
>> --- i/gitweb/Makefile
>> +++ w/gitweb/Makefile
>> @@ -18,7 +18,7 @@ RM ?= rm -f
>>  INSTALL ?= install
>>
>>  # default configuration for gitweb
>> -GITWEB_CONFIG = gitweb_config.perl
>> +GITWEB_CONFIG = ./gitweb_config.perl
>>  GITWEB_CONFIG_SYSTEM = /etc/gitweb.conf
>>  GITWEB_CONFIG_COMMON = /etc/gitweb-common.conf
>>  GITWEB_HOME_LINK_STR = projects
>>
>> but that does not help if someone overrides GITWEB_CONFIG, and besides,
>> it would be nicer to avoid the possibility of an @INC search altogether.
>> ...
> Just:
>
>     local @INC = '.';
>     do 'FILE.pl';
>
> Would do the same thing, but seems like a more indirect way to do it if
> all we want is ./ anyway.

Yeah, it does look indirect.  Despite what you said, it also would
support users giving an absolute path via GITWEB_CONFIG.

With "use File::Spec", perhaps something like this?

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 2594a4badb..239e7cbc25 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -719,6 +719,10 @@ sub filter_and_validate_refs {
 sub read_config_file {
 	my $filename = shift;
 	return unless defined $filename;
+
+	$filename = File::Spec->catfile(".", $filename)
+		unless File::Spec->file_name_is_absolute($filename);
+
 	# die if there are errors parsing config file
 	if (-e $filename) {
 		do $filename;

^ permalink raw reply related

* Re: Any way to make git-log to enumerate commits?
From: Junio C Hamano @ 2018-12-06  0:31 UTC (permalink / raw)
  To: Konstantin Khomoutov; +Cc: Konstantin Kharlamov, git
In-Reply-To: <20181205145419.vbbaghzzrnceez45@tigra>

Konstantin Khomoutov <kostix@bswap.ru> writes:

> On Wed, Dec 05, 2018 at 05:22:14PM +0300, Konstantin Kharlamov wrote:
>
>> It would be great if git-log has a formatting option to insert an
>> index of the current commit since HEAD.
>> 
>> It would allow after quitting the git-log to immediately fire up "git
>> rebase -i HEAD~index" instead of "git rebase -i
>> go-copy-paste-this-long-number-id".
>
> This may have little sense in a general case as the history maintained
> by Git is a graph, not a single line. Hence your prospective approach
> would only work for cases like `git log` called with the
> "--first-parent" command-line option.

I do not see why the "name each rev relative to HEAD" formatting
option cannot produce HEAD^2~2 etc.

It would be similar to "git log | git name-rev --stdin" but I do not
offhand recall if we had a way to tell name-rev to use only HEAD as
the anchoring point.

^ permalink raw reply


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