Trying to use insteadOf trick to use different SSH keys for separate github accounts - not working

[Asfand Qazi]

Hello,

Here's what I'm trying to do, that i need your help with to make work:

I have 2 github accounts, each with their own SSH key: my home account
(default SSH key) and my work account (alternative SSH key). I can
create a virtual hostname in my ~/.ssh/config like so:

Host work.github.com
 Hostname github.com
 IdentityFile ~/.ssh/id_rsa-work

and then clone repos with:

git clone g...@work.github.com/MyCompany/la-repo.git

However, I'm trying to use Go, which needs to access every repo host
as 'github.com', and won't support my little SSH hostname trick by
default.

I found out about the 'insteadOf' setting, and thought it would work.
So I added this to my global git config:

[url "g...@github.com:MyCompany/"]
insteadOf = g...@work.github.com:MyCompany/

and left the SSH hostname setting where it was. Then I tried doing:

git clone git:github.com/MyCompany/la-repo.git

But it won't work. With GIT_TRACE=2, I get:

$ GIT_TRACE=2 git clone g...@github.com:MyCompany/la-repo.git
07:46:27.627557 git.c:344   trace: built-in: git 'clone'
'g...@github.com:MyCompany/la-repo.git'
Cloning into 'la-repo'...
07:46:27.629623 run-command.c:626   trace: run_command: 'ssh'
'g...@github.com' 'git-upload-pack '\''MyCompany/la-repo.git'\'''
ERROR: Repository not found.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.


Can anyone help me with this?

Thanks

Regards,
 Asfand

Re: What's cooking in git.git (Dec 2017, #03; Wed, 13)

[Kaartic Sivaraam]

On Friday 15 December 2017 02:38 AM, Junio C Hamano wrote:

Junio C Hamano  writes:


I think you only sent 3/3 in newer rounds, which made it not to
apply to my tree.  If you meant to drop changes in 1/3 and 2/3,
perhaps because they were needless churn, then 3/3 needs to be
updated not to depend on the changes these two made.


Here is what I reconstructed to suit my taste better ;-)



Looks good to me except that your v4 3/3 seems to be spawning a subshell 
is an oversight, I guess? Also, I guess it would be better if there was 
comment in the code saying 'branch_nam' might hold a commit/branch name. 
No worries I'll send a v5 to that thread based on your v4.


Thanks,
Kaartic

Re: [PATCH v4 2/2] Doc/check-ref-format: clarify information about @{-N} syntax

[Kaartic Sivaraam]

On Thursday 14 December 2017 11:32 PM, Junio C Hamano wrote:

Kaartic Sivaraam  writes:

Looks alright.

It was made unnecessarily harder to review because it was marked as
2/2, even though this no longer applies on top of the copy of 1/2
that was merged some time ago.


Sorry about that but I don't remember doing anything that made it not to 
apply on top of 1/2. (I just amended my changes to my topic branch. It 
can be found at [1])




I needed to find that it was rebased
on top of 'master';


I don't remember doing any rebase on top of 'master'. My topic was (and 
still is) based on the 'master' when it was pointing at 89ea799ff (Sync 
with maint, 2017-11-15). Anyways, it's my mistake as I didn't specify 
the branch on which I based this. Sorry about that.





Also re-wrapping the lines only to squeeze in "but be cautious..."
and replace s/branch/checkout/ in a few places did not help to make
it easy to spot what's changed.



I expected this would happen but I thought the line shouldn't grew too 
much so that they have to re-wrapped. Seems it would have been better if 
I did the re-wrapping as a follow-up commit (didn't strike me then).




This part looked a bit strange.


+it can be used as a valid branch name e.g. when creating a new branch
+(but be cautious when using the previous checkout syntax; it may refer
+to a detached HEAD state). The rule `git check-ref-format --branch


I think "e.g. when creating a new branch" is a parenthetical remark,
so it should be inside parenthesis.


It was. I brought them out to introduce the parenthetical warning. I'll 
send a v5 by putting the remark back inside parantheses and bringing the 
warning out. If it's not ok, let me know. I'll also try to do the 
re-wrapping as a separate cleanup patch.




 As the last three lines in the
new text (quoted above) already warns that it may not be a branch name,
I am not sure if the "but be cautious" adds much value, though.



That warning was for the impatient readers, who might want to find quick 
answers as to why they saw an odd behaviour (check-ref-froamt --branch 
not failing for a commit object name) (or) those who would want to use 
'check-ref-format --branch' but do not find time to read the whole 
paragraph.

Re: [PATCH 2/2] fsmonitor: Store fsmonitor bitmap before splitting index

[Alex Vandiver]

On Mon, 13 Nov 2017, Ben Peart wrote:
> Why do you redirect stdout to stderr and then and perform an "echo"
> afterwards?  I don't understand what benefit that provides.  I removed this
> logic and the test still passes so am confused as to what its purpose is.

Ah -- the "echo" was purely to clean up STDERR as I was running the
test interactively.  It serves no purpose, which is why it was hard
to understand its benefit. :)

Apologies for missing this (and in not replying here earlier!).  I'll
send a commit that drops these.
 - Alex

> > +# test that splitting the index dosn't interfere
> > +test_expect_success 'splitting the index results in the same state' '
> > +   write_integration_script &&
> > +   dirty_repo &&
> > +   git update-index --fsmonitor  &&
> > +   git ls-files -f >expect &&
> > +   test-dump-fsmonitor >&2 && echo &&
> > +   git update-index --fsmonitor --split-index &&
> > +   test-dump-fsmonitor >&2 && echo &&
> > +   git ls-files -f >actual &&
> > +   test_cmp expect actual
> > +'
> > +
> >   test_done

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

[Jonathan Nieder]

Hi Simon,

Simon Doodkin wrote:

> please develop a new feature, git "cp" like there is git mv tomovefile1 
> tofile2
> (to save space).
>
> there is a solution in https://stackoverflow.com/a/44036771/466363
> however, it is not single easy command.

This sounds like a reasonable thing to add.  See builtin/mv.c for how
"git mv" works if you're looking for inspiration.

cmd_mv in that file looks rather long, so I'd also be happy if someone
interested refactors to break it into multiple self-contained pieces
for easier reading (git mostly follows
https://www.kernel.org/doc/html/latest/process/coding-style.html#functions).

Thanks,
Jonathan

Re: Git Hooks

[Satyakiran Duggina]

Thanks, Bryan.

To give the code pullers a chance to review, can we not have a
`trusted-hooks: default` and `trusted-SHA: ` field in .git/.
I'm assuming githooks/ are source tracked here.

When developer tries to execute `git commit`, git can ask developer to
change `trusted-hooks` field to true or false. Let's say developer
sets it to true, git can record the SHA. If any latest pull has the
hooks changed, git can revert the `trusted-hook` to default.

This way there is not much hassle for developers to manually copy
hooks all the time. And at the same time, they are not running scripts
that they haven't reviewed.

Will this work?



On Fri, Dec 15, 2017 at 11:23 AM, Bryan Turner  wrote:
> On Fri, Dec 15, 2017 at 11:12 AM, Satyakiran Duggina
>  wrote:
>> I see that `git init` creates a .git directory and hooks are to be
>> placed in that directory and these hooks are not tracked by version
>> control. To achieve tracked hooks, either each developer has to copy
>> the hooks or use tools like overcommit, pre-commit, husky etc.
>>
>> I'm wondering why hooks are not made external like .gitignore. I guess
>> it would be better to have two git configuration directories in a
>> repo, one hosting all the metadata managed by git and the other with
>> user configured data (hooks, ignore/exclude, repo config etc).
>
> Hooks are not external because they're not trusted. It essentially
> amounts to allowing someone to download an arbitrary script or program
> onto your computer which you then execute. It's extremely unsafe, and
> is intentionally not possible. To get hooks in your instance, you have
> to _manually_ install them. This gives you a chance to _review_ them
> before they start executing on your system. Any other approach and the
> hooks become an attack vector.
>
>>
>> Kindly let me know why the current design choice is made and if the
>> proposed change would introduce unseen issues.
>>
>>
>> Thanks,
>> Satya
>
> Hope this helps!
> Bryan Turner



-- 
Regards & Thanks
Satya Kiran Duggina

Re* [PATCH v2 2/2] t: add tests for pull --verify-signatures

[Junio C Hamano]

Junio C Hamano  writes:

> Interestingly, the tests that do expect 'git pull' to succeed
> protect themselves with "when-finished" mechanism correctly [*1*],
> like so:
>
>> +test_expect_success GPG 'pull signed commit with --verify-signatures' '
>> +test_when_finished "git checkout initial" &&
>> +git pull --verify-signatures signed >pulloutput &&
>> +test_i18ngrep "has a good GPG signature" pulloutput
>> +'
>> +
>
> Other than that, looked nicely done.  Thanks.

Here is what I would propose as a follow-up polishing.

-- >8 --
Subject: [PATCH 3/2] t5573: clean up after unexpected success of 'pull', too

The previous step added test_when_finished to tests that run 'git
pull' with expectation of success, so that the test after them can
start from a known state even when their 'git pull' invocation
unexpectedly fails.  However, tests that run 'git pull' expecting
it not to succeed forgot to protect later tests the same way---if
they unexpectedly succeed, the test after them would start from an
unexpected state.

Reset and checkout the initial commit after all these tests, whether
they expect their 'git pull' invocations to succeed or fail.

Signed-off-by: Junio C Hamano 
---
 t/t5573-pull-verify-signatures.sh | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/t5573-pull-verify-signatures.sh 
b/t/t5573-pull-verify-signatures.sh
index 8ae331f40e..9594e891f4 100755
--- a/t/t5573-pull-verify-signatures.sh
+++ b/t/t5573-pull-verify-signatures.sh
@@ -43,33 +43,36 @@ test_expect_success GPG 'create repositories with signed 
commits' '
 '
 
 test_expect_success GPG 'pull unsigned commit with --verify-signatures' '
+   test_when_finished "git reset --hard && git checkout initial" &&
test_must_fail git pull --ff-only --verify-signatures unsigned 
2>pullerror &&
test_i18ngrep "does not have a GPG signature" pullerror
 '
 
 test_expect_success GPG 'pull commit with bad signature with 
--verify-signatures' '
+   test_when_finished "git reset --hard && git checkout initial" &&
test_must_fail git pull --ff-only --verify-signatures bad 2>pullerror &&
test_i18ngrep "has a bad GPG signature" pullerror
 '
 
 test_expect_success GPG 'pull commit with untrusted signature with 
--verify-signatures' '
+   test_when_finished "git reset --hard && git checkout initial" &&
test_must_fail git pull --ff-only --verify-signatures untrusted 
2>pullerror &&
test_i18ngrep "has an untrusted GPG signature" pullerror
 '
 
 test_expect_success GPG 'pull signed commit with --verify-signatures' '
-   test_when_finished "git checkout initial" &&
+   test_when_finished "git reset --hard && git checkout initial" &&
git pull --verify-signatures signed >pulloutput &&
test_i18ngrep "has a good GPG signature" pulloutput
 '
 
 test_expect_success GPG 'pull commit with bad signature without verification' '
-   test_when_finished "git checkout initial" &&
+   test_when_finished "git reset --hard && git checkout initial" &&
git pull --ff-only bad 2>pullerror
 '
 
 test_expect_success GPG 'pull commit with bad signature with 
--no-verify-signatures' '
-   test_when_finished "git checkout initial" &&
+   test_when_finished "git reset --hard && git checkout initial" &&
test_config merge.verifySignatures true &&
test_config pull.verifySignatures true &&
git pull --ff-only --no-verify-signatures bad 2>pullerror
-- 
2.15.1-558-ged696bbdd8

Re: Question about the ahead-behind computation and status

[Derrick Stolee]

On 12/15/2017 1:30 PM, Junio C Hamano wrote:

Derrick Stolee  writes:


The biggest reason for the 20 seconds is not just the number of
commits in the ahead/behind but how many commits are walked (including
common to both branches) before paint_down_to_common() breaks its
while loop due to queue_has_nonstale().

Hmm, queue_has_nonstale() looks to see if any element is not STALE
(where the definition of STALE is "known to be a common ancestor")
by potentially checking all elements in the queue.  I wonder if we
have an opportunity for a trivial optimization?  When the caller
knows that it dug one level and added the parents that are not
stale, it does not have to ask queue_has_nonstale() if there is any
non stale element, for example.


I thought this, too, but my tracing did not show significant time spent 
in this method. 99% of the time is spent unzipping and parsing commits.


If this was taking too long, then we could track a minimum timestamp for 
a commit that entered the queue in a non-stale state, but this will 
delay the termination condition a bit since commits can be marked stale 
after they enter the queue.



What do you exactly mean by "not just the number of commits in the
ahead/behind"?  Aren't the number of these commits pretty much
proportional to the number of commits we need to paint down to
common ancestors?  Is the latter a lot larger than the former
(i.e. are we somehow not stopping when we _could_ notice that we
can with better information)?




With the wide history, there is actually a large set of commits that are 
in the common history but have newer commit times than the oldest commit 
in only one history. Consider the following ASCII art:


  A
  |
  1
  |
  2
  |
  3
  |\
  4 B
  |\|
  5 C
  |\|
  6 D
  |\|
   .
   .
   .
  |\|
  N Z
  |/
  0

Between A and B, A is ahead by commits {A,1,2,3,4,5,6,...,N}. Meanwhile, 
commits B,C,D,...,Z are in the common history, but still must be walked.


Now imagine these two sets are actually much MUCH wider (thousands of 
commits that are pairwise non-ancestors). This causes the number of 
walked commits to be larger than just the number of commits in the 
symmetric difference of the histories.


Unfortunately, generation numbers are not the only optimization needed 
to make this call be sub-100ms. A graph data structure that lists the 
edges between commits would prevent the time spent in zlib decompressing 
and parsing commits. I'm working on investigating how such a data 
structure (and corresponding file format) could integrate in the 
commit-walking code.


Thanks,
-Stolee

Re: [PATCH] git-svn: convert CRLF to LF in commit message to SVN

[Junio C Hamano]

"Bennett, Brian"  writes:

> Thank you all for your guidance,
>
> I have completed my test this morning with the patch and the 'git
> svn dcommit' is now SUCCESSFUl!

Thanks, all.

Re: "git describe" documentation and behavior mismatch

[Junio C Hamano]

Daniel Knittl-Frank  writes:

> Forget the above patch. I should compile my code after refactoring ...
>
> Here is the fixed version.
>
> -- >8 --
>
> From 8203bd0ad5baab7024ebff597c9f35a0250d09ff Mon Sep 17 00:00:00 2001
> From: Daniel Knittl-Frank 
> Date: Mon, 11 Dec 2017 19:24:54 +0100
> Subject: [PATCH] Prepend "tags/" when describing tags with embedded name
>
> Signed-off-by: Daniel Knittl-Frank 
> ---
>  builtin/describe.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)

I think the code makes sense, but it won't be understandable by
those who do not know what you discussed in the original thread.

A proper commit log message, with a new test or two in t6120, would
be an appropriate way to fix that.

Care to follow through, along the lines in
Documentation/SubmittingPatches?

Thanks.

> diff --git a/builtin/describe.c b/builtin/describe.c
> index e14e162ef6..9da6d85ea3 100644
> --- a/builtin/describe.c
> +++ b/builtin/describe.c
> @@ -271,10 +271,13 @@ static void display_name(struct commit_name *n)
>  n->name_checked = 1;
>  }
>
> -if (n->tag)
> +if (n->tag) {
> +if (all)
> +printf("tags/");
>  printf("%s", n->tag->tag);
> -else
> +} else {
>  printf("%s", n->path);
> +}
>  }
>
>  static void show_suffix(int depth, const struct object_id *oid)
> -- 
> 2.15.GIT

Re: Git Hooks

[Bryan Turner]

On Fri, Dec 15, 2017 at 11:12 AM, Satyakiran Duggina
 wrote:
> I see that `git init` creates a .git directory and hooks are to be
> placed in that directory and these hooks are not tracked by version
> control. To achieve tracked hooks, either each developer has to copy
> the hooks or use tools like overcommit, pre-commit, husky etc.
>
> I'm wondering why hooks are not made external like .gitignore. I guess
> it would be better to have two git configuration directories in a
> repo, one hosting all the metadata managed by git and the other with
> user configured data (hooks, ignore/exclude, repo config etc).

Hooks are not external because they're not trusted. It essentially
amounts to allowing someone to download an arbitrary script or program
onto your computer which you then execute. It's extremely unsafe, and
is intentionally not possible. To get hooks in your instance, you have
to _manually_ install them. This gives you a chance to _review_ them
before they start executing on your system. Any other approach and the
hooks become an attack vector.

>
> Kindly let me know why the current design choice is made and if the
> proposed change would introduce unseen issues.
>
>
> Thanks,
> Satya

Hope this helps!
Bryan Turner

Re: Need help migrating workflow from svn to git.

[Igor Djordjevic]

On 15/12/2017 20:09, Junio C Hamano wrote:
> 
> > Junio, what about consecutive runs, while merge conflicts are still 
> > unresolved?
> 
> The impression I got was that the original running with svn does not
> deal with conflicting situation anyway, so I did not think about it
> at all, and I personally do not care ;-)

Heh, fair enough :) Though I was genuinely interested if there is a 
better way to accomplish it than `git add -u && git reset`, but I 
guess I can take that as "whatever works" ;)

Git Hooks

[Satyakiran Duggina]

I see that `git init` creates a .git directory and hooks are to be
placed in that directory and these hooks are not tracked by version
control. To achieve tracked hooks, either each developer has to copy
the hooks or use tools like overcommit, pre-commit, husky etc.

I'm wondering why hooks are not made external like .gitignore. I guess
it would be better to have two git configuration directories in a
repo, one hosting all the metadata managed by git and the other with
user configured data (hooks, ignore/exclude, repo config etc).

Kindly let me know why the current design choice is made and if the
proposed change would introduce unseen issues.


Thanks,
Satya

Re: Need help migrating workflow from svn to git.

[Junio C Hamano]

Igor Djordjevic  writes:

> Junio, what about consecutive runs, while merge conflicts are still 
> unresolved?

The impression I got was that the original running with svn does not
deal with conflicting situation anyway, so I did not think about it
at all, and I personally do not care ;-)

Frau Ruth Michael

[Mrs Ruth Michael]

Mein Liebster im Herrn,

Grüße an Sie im Namen unseres Herrn Jesus Christus. Es war nach dem Durchlaufen 
Ihres Profils. Ich bin bewegt, Sie mit diesem wichtigen Thema in Kontakt zu 
bringen, mit dem Glauben und der Zuversicht, dass Sie ein guter Assistent 
dieses Werkes Gottes sein werden. Zunächst möchte ich mich Ihnen vorstellen. 
Ich bin Mrs. Ruth Michael aus Kuwait. Ich war mit dir verheiratet, Peter 
Michael. Wer hat 15 Jahre lang mit der kuwaitischen Botschaft in Côte d'Ivoire 
gearbeitet, bevor er 2010 starb?

Wir waren 13 Jahre ohne Kind verheiratet. Er starb nach einer kurzen Krankheit, 
die nur vier Tage dauerte. Bevor wir starben, waren wir beide stark gläubig. 
Seit seinem Tod habe ich beschlossen, nicht wieder zu heiraten oder ein Kind 
aus meiner Heirat zu bekommen, gegen das die Bibel ist. Als mein verstorbener 
Ehemann noch lebte, hinterlegte er den Betrag von 10,5 Millionen Dollar in 
einer Bank hier in Abidjan, dem ökonomischen Kapital der Elfenbeinküste für das 
Sorgerecht. Derzeit hat mein Arzt bestätigt, dass ich eine schwere Krankheit 
habe, die Krebserkrankung ist.

Was mich am meisten stört, ist mein Schlaganfall. Haven hat meine Bedingung 
erfüllt. Ich habe beschlossen, diesen Fonds an eine Gemeinde oder Einzelperson 
zu spenden, die dieses Geld in der Art und Weise verwenden wird, wie ich es 
hier unterrichten werde. Ich möchte eine Kirche, die diesen Fonds für 
Waisenhäuser nutzt, den Witwen hilft, das Wort Gottes verbreitet und sich 
bemüht, das Haus Gottes aufrechtzuerhalten. Die Bibel hat uns verständlich 
gemacht, dass die Hand, die gibt, gesegnet ist, ich habe diese Entscheidung 
getroffen, weil ich keinen Sohn habe, der dieses Geld erben wird, und die 
Verwandten meines Mannes sind keine Christen und ich möchte nicht, dass die 
Bemühungen meines Mannes von Ungläubigen benutzt werden.

Ich will keine Situation, in der dieses Geld gottlos eingesetzt wird. Darum 
treffe ich diese Entscheidung. Ich habe keine Angst vor dem Tod, also weiß ich, 
wohin ich gehe. Ich weiß, dass ich im Herzen des Herrn sein werde. Exodus 14 VS 
14 besagt, dass Sie meinen Fall bekämpfen werden und ich werde meinen Frieden 
behalten. Sobald ich Ihre Antwort erhalten habe, werde ich Ihnen hier in 
Abidjan den Kontakt zur Sicherheitsfirma geben. Ich möchte, dass du und die 
Kirche immer für mich beten, weil du mein Pastor bist. Mein Glück ist, dass ich 
ein Leben eines würdigen Christen gelebt habe. Wer dem Herrn dienen will, muss 
ihm im Geist und in der Wahrheit dienen.

Sei bitte immer dein Leben lang fromm. Jede Verzögerung in Ihrer Antwort wird 
mir Raum geben, eine andere Kirche oder Einzelperson für diesen gleichen Zweck 
zu beschaffen. Bitte versichere mir, dass du nach dem handeln wirst, was ich 
hier gesagt habe.

Warten auf Ihre Antwort.

Bleibe gesegnet im Herrn.

Dein in Christus,
Frau Ruth Michael

Re: Need help migrating workflow from svn to git.

[Igor Djordjevic]

On 15/12/2017 17:33, Junio C Hamano wrote:
> 
> $ git fetch  
> $ git checkout -m -B  FETCH_HEAD

... aaand that`s how you do it[1] without a temporary branch :)

Junio, what about consecutive runs, while merge conflicts are still 
unresolved?

Seeing Josef having a pretty relaxed flow, and his cron job running 
every 15 minutes, would adding something like:

$ git add -u
$ git reset

... to the mix, to "silence" actually still unresolved merge 
conflicts, making next script execution possible, make sense?

Yes, `git diff` won`t be the same as if conflicts were still in, but 
it might be worth it in this specific case, conflicting parts still 
easily visible between conflict markers.

Regards, Buga

[1] On 15/12/2017 19:24, Igor Djordjevic wrote:
> 
> git checkout -b temp &&   #1
> git fetch &&  #2
> git branch -f master origin/master && #3
> git checkout -m master && #4
> git add -u && #5
> git reset &&  #6
> git branch -d temp#7
> 
> Explanation:
>  1. Create temporary branch where we are, switching to it, so we can 
> update "master" without local modifications
>  2. Fetch latest updates
>  3. Update "master" to fetched "origin/master"
>  4. Switch to updated "master", merging local modifications
>  5. Mark any pending merge conflicts as resolved by staging them...
>  6. ... and unstage them right away
>  7. Delete temporary branch
> 
> Step (4) is what merges your local modifications with remote updates 
> (leaving conflicts, if any), where steps (5) and (6) are not needed 
> for a single run, but in case you don`t resolve conflicts before next 
> cron job executes this script again, step (1) will now fail without 
> them because of (still) unresolved merge conflicts.
> 
> So, as you seem to be pretty at ease with your flow, you might prefer 
> leaving those two steps (5, 6) in.

Re: Question about the ahead-behind computation and status

[Junio C Hamano]

Derrick Stolee  writes:

> The biggest reason for the 20 seconds is not just the number of
> commits in the ahead/behind but how many commits are walked (including
> common to both branches) before paint_down_to_common() breaks its
> while loop due to queue_has_nonstale().

Hmm, queue_has_nonstale() looks to see if any element is not STALE
(where the definition of STALE is "known to be a common ancestor")
by potentially checking all elements in the queue.  I wonder if we
have an opportunity for a trivial optimization?  When the caller
knows that it dug one level and added the parents that are not
stale, it does not have to ask queue_has_nonstale() if there is any
non stale element, for example.

What do you exactly mean by "not just the number of commits in the
ahead/behind"?  Aren't the number of these commits pretty much
proportional to the number of commits we need to paint down to
common ancestors?  Is the latter a lot larger than the former
(i.e. are we somehow not stopping when we _could_ notice that we
can with better information)?

Re: Need help migrating workflow from svn to git.

[Igor Djordjevic]

Hi Josef,

Thank you for your patient answers. From what you said here and in 
that other reply[1], it looks like you know what you`re doing, you`re 
aware of circumstances, and you still prefer doing it that way.

So, here it goes... :)

On 15/12/2017 13:47, Josef Wolf wrote:
> 
> > I`m thinking of a workflow involving (scripted) creation of a 
> > temporary branch at fetched remote branch position, and using 
> > something like `git checkout --merge ` to merge your 
> > local modifications to latest changes fetched from remote (ending
> > up with conflicts inside working tree, if any),
> 
> But this would require local modifications to be committed?
 
Nope :) Here`s a script you can test to see if it works for you, 
simulating `svn update` (at least how I perceived it).

Feel free to adapt as you feel like it (I used local "master" branch 
and remote "origin/master", for example), or to speak up if any 
additional info is needed.

git checkout -b temp &&   #1
git fetch &&  #2
git branch -f master origin/master && #3
git checkout -m master && #4
git add -u && #5
git reset &&  #6
git branch -d temp#7

Explanation:
 1. Create temporary branch where we are, switching to it, so we can 
update "master" without local modifications
 2. Fetch latest updates
 3. Update "master" to fetched "origin/master"
 4. Switch to updated "master", merging local modifications
 5. Mark any pending merge conflicts as resolved by staging them...
 6. ... and unstage them right away
 7. Delete temporary branch

Step (4) is what merges your local modifications with remote updates 
(leaving conflicts, if any), where steps (5) and (6) are not needed 
for a single run, but in case you don`t resolve conflicts before next 
cron job executes this script again, step (1) will now fail without 
them because of (still) unresolved merge conflicts.

So, as you seem to be pretty at ease with your flow, you might prefer 
leaving those two steps (5, 6) in.

This does seem ugly and hacky, but if it works for you, I don`t judge :) 
Please note that there might be better ways to accomplish this, I 
repeat, I`m not an expert, but hopefully this could do the job.

Also, if I missed something, I hope someone will correct me.

Regards, Buga

[1] https://public-inbox.org/git/20171215130645.gd18...@raven.inka.de/

Re: [PATCH v2 0/2] Offer more information in `git version --build-options`'s output

[Junio C Hamano]

Eric Sunshine  writes:

> On Fri, Dec 15, 2017 at 12:45 PM, Junio C Hamano  wrote:
>> Johannes Schindelin  writes:
>>>   no commit associated with this build
>>
>> I find this output somewhat klunky for machine parsing (and it is
>> inconsistent with the style used for "sizeof-long", which hints that
>> these are "  " lines where whitespaces are
>> avoided in a ), but hopefully this is primarily for human
>> consumption and scrypts that are trying to find a specific piece of
>> information would know how to use 'grep', so the inconsistency does
>> not make much of a difference in practice anyway.
>
> Simply omitting that line from the output of --build-options would
> also be an option if the commit can not be determined...

What we see in Dscho's patch is better than that option, though.  At
least, explicitly checking for "no commit associated" would tell you
if the version of Git is too old to have "built from commit:" line
or it is new enough but was built from a tarball.

Re: [PATCH v2 0/2] Offer more information in `git version --build-options`'s output

[Eric Sunshine]

On Fri, Dec 15, 2017 at 12:45 PM, Junio C Hamano  wrote:
> Johannes Schindelin  writes:
>>   no commit associated with this build
>
> I find this output somewhat klunky for machine parsing (and it is
> inconsistent with the style used for "sizeof-long", which hints that
> these are "  " lines where whitespaces are
> avoided in a ), but hopefully this is primarily for human
> consumption and scrypts that are trying to find a specific piece of
> information would know how to use 'grep', so the inconsistency does
> not make much of a difference in practice anyway.

Simply omitting that line from the output of --build-options would
also be an option if the commit can not be determined...

Re: [PATCH v2 0/2] Offer more information in `git version --build-options`'s output

[Junio C Hamano]

Johannes Schindelin  writes:

> - when no commit can be determined, it now says
>
>   no commit associated with this build
>   instead of
>
>   built from commit: (unknown)

I find this output somewhat klunky for machine parsing (and it is
inconsistent with the style used for "sizeof-long", which hints that
these are "  " lines where whitespaces are
avoided in a ), but hopefully this is primarily for human
consumption and scrypts that are trying to find a specific piece of
information would know how to use 'grep', so the inconsistency does
not make much of a difference in practice anyway.

Queued.  Thanks.

Re: [PATCH v3] Makefile: replace perl/Makefile.PL with simple make rules

[Ævar Arnfjörð Bjarmason]

On Fri, Dec 15, 2017 at 11:35 AM, Michael J Gruber  wrote:
> Ævar Arnfjörð Bjarmason venit, vidit, dixit 12.12.2017 23:26:
>>
>> On Tue, Dec 12 2017, Randall S. Becker jotted:
>>
>>> -Original Message-
>>> On December 10, 2017 4:14 PM, Ævar Arnfjörð Bjarmason wrote:
>>> Subject: [PATCH v3] Makefile: replace perl/Makefile.PL with simple make 
>>> rules
>>>
 Replace the perl/Makefile.PL and the fallback perl/Makefile used under 
 NO_PERL_MAKEMAKER=NoThanks with a much simpler implementation heavily 
 inspired by how the i18n infrastructure's build process works[1].
 The reason for having the Makefile.PL in the first place is that it was 
 initially[2] building a perl C binding to interface with libgit, this 
 functionality, that was removed[3] before Git.pm ever made it to the 
 master branch.
>>> 
>>>
>>> I would like to request that the we be careful that the git builds do not 
>>> introduce arbitrary dependencies to CPAN. Some platforms (I can think of 
>>> one off the top, being NonStop) does not provide for arbitrary additions to 
>>> the supplied perl implementation as of yet. The assumption about being able 
>>> to add CPAN modules may apply on some platforms but is not a general 
>>> capability. I am humbly requesting that caution be used when adding 
>>> dependencies. Being non-$DAYJOB responsible for the git port for NonStop, 
>>> this scares me a bit, but I and my group can help validate the available 
>>> modules used for builds.
>>>
>>> Note: we do not yet have CPAN's SCM so can't and don't use perl for access 
>>> to git anyway - much that I've tried to change that.
>>>
>>> Please keep build dependencies to a minimum.
>>>
>>> Thanks for my and my whole team.
>>
>> I think you should be happy with this patch then, and it doesn't add any
>> more CPAN dependency than before, and sets up a framework (as discussed
>> in [1]) where we can use more CPAN modules while not requiring packagers
>> such as yourself to package CPAN modules.
>>
>> However, it doesn't sound believable to me that even on NonStop you
>> can't install any CPAN modules whatsoever.
>>
>> That would also mean that this patch doesn't work for you, because it
>> means that you either don't have anything resembling a hierarchical
>> filesystem on which git can be installed in the first place (in which
>> case it wouldn't work), or perl doesn't have an @INC to search through
>> perl libs on on NonStop. What does:
>>
>> perl -V
>>
>> Return for you on that system?
>>
>> If this patch works, and if at the bottom of `perl -V` you see some
>> directories which you could write a package to drop some static *.pm
>> files, then you can grab a *.tar.gz from CPAN such as the one for
>> Error.pm[2] and arrange for the *.pm files contained within its lib/
>> directory to be dropped into one of those @INC directories.
>>
>> It may be that some aspect of the CPAN toolchain is broken for you, or
>> even ExtUtils::MakeMaker, but you typically don't need that to package
>> non-XS perl modules, certainly not any of the ones we've discussed
>> possibly bundling up in git.git on-list recently. As a (very occasional)
>> contributor to perl.git I'd be interested to know if that's what you
>> mean is broken, and if so see if it could be fixed for you.
>>
>> 1. 

Re: [PATCH v2 4/4] t/Makefile: introduce TEST_SHELL_PATH

[Junio C Hamano]

Jeff King  writes:

> I think (4) and (5) are the only things that actually change the
> behavior in a meaningful way. But they're a bit more hacky and
> repetitive than I'd like. Especially given that I'm not really sure
> we're solving an interesting problem. I'm happy enough with the patch as
> shown, and I do not recall anybody complaining about the current
> behavior of these options.

OK.  Thanks for thinking it through.

>> There is a long outstanding NEEDSWORK comment in help.c that wonders
>> if we want to embed contents from GIT-BUILD-OPTIONS in the resulting
>> binary, and the distinction Dscho brought up between "build" and
>> "test" phases would start to matter even more once we go in that
>> direction.
>
> I guess you're implying having a GIT-BUILD-OPTIONS and a
> GIT-TEST-OPTIONS here.

I admit that my thinking did not go that far to introduce the
latter, as "git version --how-did-we-build-this-exact-git" only
needs the former.  But you're right that some information given
at the top-level must be stored somewhere t/test-lib.sh reads in
order to allow us run tests from outside Makefile (your point 1.)

Re: Need help migrating workflow from svn to git.

[Junio C Hamano]

Josef Wolf  writes:

> With git, by contrast, this won't work. Git will refuse to pull anything as
> long as there are ANY local modifications. The cron job would need to
>
>git stash
>git pull
>git stash pop

I'd assume that this "pull" is expected to be fast-forward, as
otherwise you have no way of dealing with conflicted merges.

> But this will temporarily remove my local modifications. If I happen to do
> a test run at this time, the test run would NOT contain the local
> modifications which I was about to test. Even worse: if I happen to save
> one of the modified files while the modifications are in the stash, the
> "git stash pop" will definitely cause a conflict, although nothing really
> changed.
>
> So, how would I get this workflow with git? Is it possible to emulate the
> behavior of "svn update"?

You do not mind a temporary inconsistency while "svn update" runs
(it starts to update a file you may have local changes, but your
test may run while the update is in the middle of it).  So perhaps
something along the lines of this would help.  Assuming

 : the branch at the remote you are pulling from
: whatever branch you are using

are in your three-command example above:

$ git fetch  
$ git checkout -m -B  FETCH_HEAD

should give you pretty-much identical result as

$ git stash && git pull --ff-only && git stash pop

including a possible merge conflicts at 'git stash pop' stage.

Re: [PATCH v2] doc: add triangular workflow

[Matthieu Moy]

ALBERTIN TIMOTHEE p1514771  writes:

 +This workflow is commonly used on different platforms like BitBucket,
 +GitHub or GitLab which provide a dedicated mechanism for requesting 
 merges.
>>>
>>> As a user, I find it terribly frustrating when reading documentation
>>> telling me that "there's a dedicated mechanism" for something without
>>> telling me actually how to do it.
>
>>Also I think the description is backwards from end-user's point of
>>view.  It's not that it is common to use the workflow on these
>>hosting sites.  It's more like people use the workflow and adopt use
>>of these hosting sites as one building block of the workflow.
>
> I'm wondering if this sentence is really useful. It's not essential
> and it will take lot of time and space to talk about it properly.
> So, if you agree, we'll erase it.

I think it is useful. My guess is that most people don't know the
wording "triangular workflow", but most people know about GitHub for
example. See e.g.
https://trends.google.com/trends/explore?q=%22triangular%20workflow%22,github,gitlab,bitbucket

So the hope here is that the reader reading this feels "Ah, OK, this is
about how to do pull-requests on GitHub". OTOH, I wouldn't like a Git
official documentation to be biaised towards a single hosting site.

> In the case of a triangular workflow, if the project already exists,
> PUBLISH will already exist too.

No.

If the project already exists, then UPSTREAM exists, and the contributor
will create his or her own PUBLISH. There's generally two ways to do it:

* On platforms supporting this, use the platform's mechanism to create a
  fork (e.g. fork button on GitHub/GitLab's web UI).

* One can create an empty PUBLISH, clone UPSTREAM, and push to PUBLISH.

-- 
Matthieu Moy
https://matthieu-moy.fr/

Re: [PATCH v2] doc: add triangular workflow

[Matthieu Moy]

ALBERTIN TIMOTHEE p1514771  writes:

>>> +Example with master as :
>>> +===
>>> +* `git config branch.master.remote upstream`
>>> +* `git config branch.master.pushRemote origin`
>>> +===
>
>>origin is the remote you've cloned from. From the text above, I guess
>>you meant it to point to PUBLISH. But all the examples "git clone" you
>>gave are from UPSTREAM.
>
>>You're mixing the case where one "git clone"s from UPSTREAM and "git
>>remode add"s PUBLISH, and the converse. Both are possible, but the
>>"origin" remote will be different depending on which one you chose.
>
> I think I don't really get it. IMHO UPSTREAM is name from the repository
> you pull from and PUBLISH from the repositiry you push to.

In your document, you're suggesting to clone from ORIGIN, and then to
set pushRemote to origin. This means "git push" will push to ORIGIN,
which doesn't work. Actually, if one follows your instructions, upstream
and origin will point to the same remote.

Did you test your own document on a real-life example? If not, you
should do so before anything else. You should notice this kind of issues
before asking for external review.

-- 
Matthieu Moy
https://matthieu-moy.fr/

RE: [PATCH v2] doc: add triangular workflow

[ALBERTIN TIMOTHEE p1514771]

>>> +This workflow is commonly used on different platforms like BitBucket,
>>> +GitHub or GitLab which provide a dedicated mechanism for requesting merges.
>>
>> As a user, I find it terribly frustrating when reading documentation
>> telling me that "there's a dedicated mechanism" for something without
>> telling me actually how to do it.

>Also I think the description is backwards from end-user's point of
>view.  It's not that it is common to use the workflow on these
>hosting sites.  It's more like people use the workflow and adopt use
>of these hosting sites as one building block of the workflow.

I'm wondering if this sentence is really useful. It's not essential
and it will take lot of time and space to talk about it properly.
So, if you agree, we'll erase it.

>>> +If **PUBLISH** doesn't exist, a contributor can publish his own repository.
>>> +**PUBLISH** contains modifications before integration.

>I am not sure what this really means.  Isn't it the norm to create a
>place to publish your work (and only your work) for your own use?
>IOW, the above two lines solicit questions like "So... what happens
>when publish does already exist??? and where does that publish
>repository come from???"

In the case of a triangular workflow, if the project already exists,
PUBLISH will already exist too. However, if the project doesn't exist
it is at the creator charge to create it. In fact, we just explain
how doing it if the project already exist. We'll add it for the 
second case.
BTW, the line : * `git push`   is useless.


Thank you for the review

Timothée Albertin

Re: [PATCH] send-email: extract email-parsing code into a subroutine

[Matthieu Moy]

PAYRE NATHAN p1508475  writes:

>>> +sub parse_header_line {
>>> + my $lines = shift;
>>> + my $parsed_line = shift;
>>> + my $pattern = join "|", qw(To Cc Bcc);
>>
>> Nit: you may want to rename it to something more explicit, like
>> $addr_headers_pat.
>
> I find "$addr_headers_pat" too long that's why I've choose rename it
> into "$addr_pat", in addition to that, because the variable is in the
> subroutine "parse_header_line" it does not require to include
> "headers" in the variable name.

I suggested this name because $addr_pat seems to imply that this matches
an address, while it matches the _name of headers_ containing address.
But that's not terribly important, the meaning is clear by the context
anyway.

All my previous remarks have been taken into account. This is now

Reviewed-by: Matthieu Moy 

Thanks,

-- 
Matthieu Moy
https://matthieu-moy.fr/

Re: Question about the ahead-behind computation and status

[Derrick Stolee]

On 12/15/2017 10:08 AM, Jeff Hostetler wrote:

On 12/15/2017 5:08 AM, Jeff King wrote:

On Thu, Dec 14, 2017 at 04:49:31PM -0500, Jeff Hostetler wrote:

[*] Sadly, the local repo was only about 20 days out of
 date (including the Thanksgiving holidays)


Taking 20 seconds to traverse 20 days worth of history sounds pretty
awful. How many commits is it?


150,000 commits, give or take.  The graph is many thousands of lanes
wide because of the merges and that isn't helping.

(But I should give you folks lots of credit -- it *only* took 20
seconds to find, unzip and decode 150,000 commit objects and walk
the commit graph.)


To give a few more data points, I created similar situation by checking 
out a big repo I hadn't updated in three months and it was 16,000 
commits behind. That took 1.5s to calculate the ahead/behind. Moving it 
100,000 commits behind it took 5s. This repo has about 300,000 total 
commits versus the 1.5 million commits in the Windows repo.


The biggest reason for the 20 seconds is not just the number of commits 
in the ahead/behind but how many commits are walked (including common to 
both branches) before paint_down_to_common() breaks its while loop due 
to queue_has_nonstale().


Thanks,
-Stolee

[PATCH] send-email: extract email-parsing code into a subroutine

[Nathan Payre]

The existing code mixes parsing of email header with regular
expression and actual code. Extract the parsing code into a new
subroutine "parse_header_line()". This improves the code readability
and make parse_header_line reusable in other place.

"parsed_header_line()" and "filter_body()" could be used for
refactoring the part of code which parses the header to prepare the
email and send it.

In contrast to the previous version it doesn't keep the header order
and strip duplicate headers.

Signed-off-by: Nathan Payre 
Signed-off-by: Matthieu Moy 
Signed-off-by: Timothee Albertin 
Signed-off-by: Daniel Bensoussan 
Signed-off-by: Junio C Hamano 
Thanks-to: Ævar Arnfjörð Bjarmason 
---

>> +sub parse_header_line {
>> + my $lines = shift;
>> + my $parsed_line = shift;
>> + my $pattern = join "|", qw(To Cc Bcc);
>
> Nit: you may want to rename it to something more explicit, like
> $addr_headers_pat.

I find "$addr_headers_pat" too long that's why I've choose rename it
into "$addr_pat", in addition to that, because the variable is in the
subroutine "parse_header_line" it does not require to include
"headers" in the variable name.

 git-send-email.perl | 115 +++-
 1 file changed, 77 insertions(+), 38 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 2208dcc21..e6e813041 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -703,57 +703,70 @@ EOT3
do_edit($compose_filename);
}
 
-   open my $c2, ">", $compose_filename . ".final"
-   or die sprintf(__("Failed to open %s.final: %s"), 
$compose_filename, $!);
-
open $c, "<", $compose_filename
or die sprintf(__("Failed to open %s: %s"), $compose_filename, 
$!);
 
-   my $need_8bit_cte = file_has_nonascii($compose_filename);
-   my $in_body = 0;
-   my $summary_empty = 1;
if (!defined $compose_encoding) {
$compose_encoding = "UTF-8";
}
-   while(<$c>) {
-   next if m/^GIT:/;
-   if ($in_body) {
-   $summary_empty = 0 unless (/^\n$/);
-   } elsif (/^\n$/) {
-   $in_body = 1;
-   if ($need_8bit_cte) {
-   print $c2 "MIME-Version: 1.0\n",
-"Content-Type: text/plain; ",
-  "charset=$compose_encoding\n",
-"Content-Transfer-Encoding: 8bit\n";
-   }
-   } elsif (/^MIME-Version:/i) {
-   $need_8bit_cte = 0;
-   } elsif (/^Subject:\s*(.+)\s*$/i) {
-   $initial_subject = $1;
-   my $subject = $initial_subject;
-   $_ = "Subject: " .
-   quote_subject($subject, $compose_encoding) .
-   "\n";
-   } elsif (/^In-Reply-To:\s*(.+)\s*$/i) {
-   $initial_reply_to = $1;
-   next;
-   } elsif (/^From:\s*(.+)\s*$/i) {
-   $sender = $1;
-   next;
-   } elsif (/^(?:To|Cc|Bcc):/i) {
-   print __("To/Cc/Bcc fields are not interpreted yet, 
they have been ignored\n");
-   next;
+
+   my %parsed_email;
+   while (my $line = <$c>) {
+   next if $line =~ m/^GIT:/;
+   parse_header_line($line, \%parsed_email);
+   if ($line =~ /^$/) {
+   $parsed_email{'body'} = filter_body($c);
}
-   print $c2 $_;
}
close $c;
-   close $c2;
 
-   if ($summary_empty) {
+   open my $c2, ">", $compose_filename . ".final"
+   or die sprintf(__("Failed to open %s.final: %s"), $compose_filename, 
$!);
+
+
+   if ($parsed_email{'From'}) {
+   $sender = delete($parsed_email{'From'});
+   }
+   if ($parsed_email{'In-Reply-To'}) {
+   $initial_reply_to = delete($parsed_email{'In-Reply-To'});
+   }
+   if ($parsed_email{'Subject'}) {
+   $initial_subject = delete($parsed_email{'Subject'});
+   print $c2 "Subject: " .
+   quote_subject($initial_subject, $compose_encoding) .
+   "\n";
+   }
+
+   if ($parsed_email{'MIME-Version'}) {
+   print $c2 "MIME-Version: $parsed_email{'MIME-Version'}\n",
+   "Content-Type: 
$parsed_email{'Content-Type'};\n",
+   "Content-Transfer-Encoding: 
$parsed_email{'Content-Transfer-Encoding'}\n";
+   delete($parsed_email{'MIME-Version'});

Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output

[Johannes Schindelin]

Hi Gábor,

On Fri, 15 Dec 2017, SZEDER Gábor wrote:

> On Fri, Dec 15, 2017 at 1:10 PM, Johannes Schindelin
>  wrote:
> >
> >> There is a lot going on in 'run-windows-build.sh', so the output of
> >> 'set -x' might be useful or might be considered too much clutter, I
> >> don't know.  I put Dscho on Cc, I think it's mainly his call.
> >
> > Certainly it might be useful.
> >
> > However, please make sure that the secret token is not leaked that
> > way.  Like, *really* sure. Due to the failure of Git to use a portable
> > and performant test suite, it does take about 90 minutes to build and
> > test a revision, therefore it would be very easy to DOS my build
> > system, and I really, really need it not to be DOSed because I use it
> > in my day job, too.
> 
> Ugh, I was completely unaware of this issue, and the first version of
> this patch is already in 'pu'...  **runs to check the trace logs**
> 
> Great, it seems we are in luck, as the secret token was specified as an
> encrypted environment variable in git/git repository settings on Travis
> CI.  It's redacted in the trace log [...]

That's good enough.

Thanks for making sure,
Dscho

Re: What's cooking in git.git (Dec 2017, #03; Wed, 13)

[Kaartic Sivaraam]

On Thursday 14 December 2017 10:50 PM, Junio C Hamano wrote:

Kaartic Sivaraam  writes:


It seems my series that fixes an error message in 'git-rebase'[1]
(apart from a little cleanups) is missing. I guess I addressed the
issue that was raised in 3/3 as a v3 for that patch[2]. Are any more
changes needed?

[1]: <20171127172104.5796-1-kaartic.sivar...@gmail.com>
[2]: <20171201060935.19749-1-kaartic.sivar...@gmail.com>


I think you only sent 3/3 in newer rounds, which made it not to
apply to my tree.  


That's surprising, let me check.



If you meant to drop changes in 1/3 and 2/3,
perhaps because they were needless churn, then 3/3 needs to be
updated not to depend on the changes these two made.



2/3 is for sure not needless. But 1/3 might be. Though I think it 
improves the communicativity of the variable name. In case you think 
it's completely useless, then I can drop it as we won't lose much anyway ??



---
Kaartic

RE: [PATCH v2] doc: add triangular workflow

[ALBERTIN TIMOTHEE p1514771]

>> +
>> +
>> +--   -
>> +| UPSTREAM   |  maintainer   | PUBLISH   |
>> +||- - - - - - - -|   |
>> +--  <-   -
>> +  \ /
>> +   \   /
>> +fetch | \ / ^ push
>> +  v  \   /  |
>> +  \ /
>> +   -
>> +   |   LOCAL   |
>> +   -

>This kind of diagram deserves a bit of text to explain the situation.
>For example, LOCAL is local only for the contributor (the maintainer
>doesn't need to know about it for example). I'd add a sentence to
>explain that this gives the overall view on the flow, from the point
>of view of a contributor.

Ok, we'll do that

>> +* `git push`

>This will push to UPSTREAM, right?

Yes, we will specify it.

>> +Adding **UPSTREAM** remote:
>> +
>> +===
>> +`git remote add upstream `
>> +===

>In which circumstance shall one write this? If you don't say it, the
>reader will probably assume that this is to be done after the commands
>you specified right above. But then: it doesn't make sense. You've
>just cloned from UPSTREAM, you already have the UPSTREAM remote.

Indeed, we just remove it.

>> +For each branch requiring a triangular workflow, set
>> +`branch..remote` and `branch..pushRemote` to set up
>> +the **UPSTREAM** and **PUBLISH** repositories.

>This neither tells me how to set the variables, nor what the effect
>will be ("set up"?).

We'll fix that in the next patch.

>> +Example with master as :
>> +===
>> +* `git config branch.master.remote upstream`
>> +* `git config branch.master.pushRemote origin`
>> +===

>origin is the remote you've cloned from. From the text above, I guess
>you meant it to point to PUBLISH. But all the examples "git clone" you
>gave are from UPSTREAM.

>You're mixing the case where one "git clone"s from UPSTREAM and "git
>remode add"s PUBLISH, and the converse. Both are possible, but the
>"origin" remote will be different depending on which one you chose.

I think I don't really get it. IMHO UPSTREAM is name from the repository
you pull from and PUBLISH from the repositiry you push to.

>> +Making your work available
>> +~~
>> +
>> +The `git push` command sends commits to the **PUBLISH** repository and not 
>> to
>> +the **UPSTREAM** thanks to the configuration you did earlier with the
>> +`git config remote.pushdefault origin` command.

>This explanation should be next to the place where you recommend
>setting remote.pushdefault.

Done.

>> +When a contributor pushes something, the `git config push.default
>> +current` command can be used to specify that the name of the
>> +**PUBLISH** branch is the same as the name of the **LOCAL** one.

>I already said it multiple times, but I don't think it's a good idea
>to recommend changing push.default. The default, "simple", was
>specifically designed to be appropriate for triangular workflow:

  
>http://git.661346.n2.nabble.com/PATCH-0-6-push-default-in-the-triangular-world-td7589907.html
  >(PATCH 3/6 in particular)

>You may disagree with me, but then please explain your motivation (by
>replying to my messages and/or by explaining the rationale in the
>commit message).

I read this discussion and so I understand the point here. I agree we
shouldn't recommend this.

>> +=
>> +`git rev-parse --abbrev-ref @{push}`
>> +=
>> +
>> +.Display the fetch remote's name:
>> +[caption="Recipe: "]
>> +
>> +===
>> +`git rev-parse --abbrev-ref @{upstream}`
>> +===

>I don't think "rev-parse" is the best example to give.

>I use @{upstream} all the time to see what commits I have which aren't
>in upstream yet:

  >git log @{upstream}..

git log seems a better exemple.

We are ok we the rest of the review


Thank you for your time

Timothée Albertin

Re: [PATCH v3] Makefile: replace perl/Makefile.PL with simple make rules

[Todd Zullinger]

Hi Michael,

Michael J Gruber wrote:
> This patch (currently in origin/next) makes a ton of tests from our test
> suite fail for me on pretty standard systems (Fedora 27, CentOS 7.4.1708).
> 
> Is there anything I'm supposed to do differently now to make our test
> suite run? If yes then a clear and short hint in the patch description
> would me more than approriate.

Interesting.  I'll have to try building next.  I was going
to wait until the first 2.16.0 rc for a full test.

I did apply this patch on top of 2.15.1 on 12/10 and built
an rpm locally for fedora (only f26 though).  I didn't see
any test failures, which is why I thought I'd wait for
2.16.0-rc0 to test further.

FWIW, the minor spec file changes I made (against fedora's
git package master branch) are here:

https://src.fedoraproject.org/fork/tmz/rpms/git/branch/perl-makefile

The only change in the patch since I tested it is: 

+-modules += Git/Packet

in perl/Makefile.

I don't think any of these changes to the rpm spec file or
the Git/Packet addition in modules look like they'd affect
the test suite, but it's early here and I could be wrong.

I'll try to test a build of next soon to see if I get
similar failures on Fedora/CentOS.

-- 
Todd
~~
After one look at this planet any visitor from outer space would say
"I want to see the manager."
-- William S. Burroughs

Re: Question about the ahead-behind computation and status

[Jeff Hostetler]

On 12/15/2017 5:08 AM, Jeff King wrote:

On Thu, Dec 14, 2017 at 04:49:31PM -0500, Jeff Hostetler wrote:


I don't want to jump into the graph algorithm at this time,
but was wondering about adding a --no-ahead-behind flag (or
something similar or a config setting) that would disable
the a/b computation during status.

For status V2 output, we could omit the "# branch.ab x y"
line.  For normal status output, change the prose a/b
message to say something like "are [not] up to date".


Is it actually "status --porcelain=v2" that you care about here? Or is
it normal "git status"?


My primary concern is for porcelain V2 because Visual Studio
uses --branch with V2 to get the other branch details with
one process rather than two on inotify events.

Fixing/Helping normal "git status" would be nice for interactive
users.

 

Do you care about seeing the branch at all? I.e., would "--no-branch" be
a viable option (though I notice it seems to be a silent noop with the
long-form, which should probably be fixed).


VS does not use the a/b counts at all, so it is just overhead.
VS does use some of the other branch fields, so it would be nice
to still have them.

It would be nice to minimize changes to the user experience.
Not printing anything about the a/b in the normal/short/long forms
might give the user a false sense of security.  I was thinking if
we changed the message to be "you are [not] up to date", they would
still know they needed to push.

As it is, at this scale, having the message give an exact count
just isn't that useful -- who cares if you are 150,000 or 149,000
behind.

Maybe we could do something like GCC and just give up at 100
and print "you are 100+ ...".

 

If you really do want to see all branch details but just skip the
ahead/behind, then yeah, I'd agree that adding an option to do so makes
sense. Is "status" the only command that needs it? I think we do it
unconditionally with "git checkout", too.


ok. thanks. i'll look at doing that.

Yeah, "checkout" and "branch -vv" could use it too.  I haven't looked
yet to see anywhere else we might want it, but these can wait.



[*] Sadly, the local repo was only about 20 days out of
 date (including the Thanksgiving holidays)


Taking 20 seconds to traverse 20 days worth of history sounds pretty
awful. How many commits is it?


150,000 commits, give or take.  The graph is many thousands of lanes
wide because of the merges and that isn't helping.

(But I should give you folks lots of credit -- it *only* took 20
seconds to find, unzip and decode 150,000 commit objects and walk
the commit graph.)

For a true solution, I think we want to wait for client-side
generation numbers before we spend any time looking at the
algorithm.

Thanks,
Jeff

Mrs.Susan Ibrahim

[Dear beloved]

Remain Blessed,.pdf
Description: Adobe PDF document

RE: [PATCH] git-svn: convert CRLF to LF in commit message to SVN

[Bennett, Brian]

Thank you all for your guidance,

I have completed my test this morning with the patch and the 'git svn dcommit' 
is now SUCCESSFUl!

Thank you again for all of your help and I'll await to see when the patch is 
posted.

Brian Bennett | Supv System Admin & Support, TA TECH Change Mgmt/Production 
Support
o: 319-355-7602 | c: 319-533-1094
e: brian.benn...@transamerica.com | w: www.transamerica.com

Transamerica
6400 C St. SW, Cedar Rapids, IA 52404 MS-2410
Facebook | LinkedIn


-Original Message-
From: Todd Zullinger [mailto:todd.zullin...@gmail.com] On Behalf Of Todd 
Zullinger
Sent: Thursday, December 14, 2017 5:50 PM
To: Bennett, Brian 
Cc: Eric Wong ; Junio C Hamano ; 
git@vger.kernel.org
Subject: Re: [PATCH] git-svn: convert CRLF to LF in commit message to SVN

Hi Brian,

Bennett, Brian wrote:
> Thank you for your fast response,
> 
> I haven't done a build of this type before (so I could test the patch 
> first) so I'm trying to do that and get this far:
...
> I don't want to drag out testing the patch, so if either of you are 
> able to quickly guide me on what I am doing incorrectly I am willing 
> to get the build done so I can test it. If not, could one of you build 
> with the patch and somehow get that to me so I could test?

I don't know about building git for windows, but since the git-svn command is a 
perl script, it might be easier to just patch that file.  I think you can find 
the path where git-svn is installed using: git --exec-path

For this one-liner, I'd just manually apply it.

(If you want to use 'git apply' or the patch command, you'll have to edit the 
patch to adjust the name of the file, as it's git-svn.perl in the git tree.  
The .perl suffix is dropped in the installed version.)

Hopefully that makes it easier for you to test Eric's patch.

--
Todd
~~
I am in shape.  Round is a shape.

Re: Need help migrating workflow from svn to git.

[Josef Wolf]

On Fri, Dec 15, 2017 at 02:17:40AM +0100, Igor Djordjevic wrote:
> 
> This said, and without having you to change your habits too much (nor 
> use Git in possibly awkward ways), I`m thinking you may actually 
> benefit of using `git worktree add `[1] to create a 
> temporary working tree ("working copy", as you say), alongside a 
> temporary branch, where you could hack and test as much as you want, 
> unaffected by cron job updating and executing the original working 
> copy/branch (also not stepping on cron job`s toes yourself).

Interesting command. Did not know about it. Will remembre it for other use
cases!

But in this case, it is not of much use. See, I am doing system configuration
with those scripts. Having two working trees would cause the configuration to
be restored every time cron happens to start the scripts.

Doing the modifications right there where cron executes has the benefit that
cron uses the same modifications which I am using. This way, whenever cron
decides to execute, it is exactly the same as if I would do a "make run" on
the command line. Since all the scripts are designed to be idempotent,
everything works pretty much flawlessly.

-- 
Josef Wolf
j...@raven.inka.de

Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output

[SZEDER Gábor]

On Fri, Dec 15, 2017 at 1:10 PM, Johannes Schindelin
 wrote:
> Hi,
>
>> There is a lot going on in 'run-windows-build.sh', so the output of 'set
>> -x' might be useful or might be considered too much clutter, I don't
>> know.  I put Dscho on Cc, I think it's mainly his call.
>
> Certainly it might be useful.
>
> However, please make sure that the secret token is not leaked that way.
> Like, *really* sure. Due to the failure of Git to use a portable and
> performant test suite, it does take about 90 minutes to build and test a
> revision, therefore it would be very easy to DOS my build system, and I
> really, really need it not to be DOSed because I use it in my day job, too.

Ugh, I was completely unaware of this issue, and the first version of
this patch is already in 'pu'...  **runs to check the trace logs**

Great, it seems we are in luck, as the secret token was specified as an
encrypted environment variable in git/git repository settings on Travis
CI.  It's redacted in the trace log and I only see lines like these:

  Setting environment variables from repository settings
  $ export GFW_CI_TOKEN=[secure]

  +test -z [secure]

  +++curl -H 'Authentication: Bearer [secure]' --silent --retry 5
--write-out '%{HTTP_CODE}' --output /dev/fd/63
'https://git-for-windows-ci.azurewebsites.net/api/TestNow?action=trigger=pu=1229713f78cd2883798e95f33c19c81b523413fd=false'

  https://travis-ci.org/git/git/jobs/316791071

Phew.

However, I don't feel competent enough with Travis CI's encrypted
environment variables to say that this qualifies as "*really* sure"
"that the secret token is not leaked".
Anyway, note, that that '$ export GFW_CI_TOKEN=[secure]' line is already
present in all 'git/git' trace logs independently of this 'set -x'
change in question.

Gábor

Re: Need help migrating workflow from svn to git.

[Josef Wolf]

Thanks for your input, Igor!

On Thu, Dec 14, 2017 at 11:27:09PM +0100, Igor Djordjevic wrote:
> Aside "update and merge" working copy while you`re hacking on it, 
> what happens with "execute" part? It seems really strange that you 
> don`t mind cron job running the same scripts which you are actively 
> working on, thus being in an inconsistent state, if not broken, even.

In theory, you're right. In practice, problems are almost non-existent because
of:

1. Scripts are independant from each other. Would there be a problem with one
   script, the other several hundred scripts will still continue to work.
2. Most changes to existing scripts are just minor tweaks. Not much room to
   wind up.
3. When new scripts/features are introduced, it is usually to some aspect that
   were non-existing before. Breaking something that did not work before is
   not a big issue.
4. Even IF cron happens to execute something half-baked, most of the time it
   will give a syntax error. The effect is as if the cron job would have been
   stopped entirely
5. When there's a major refactoring to be done where some problems are to
   expected, then the cron entry is disabled.

So, in practice, it's pretty much a non-issue.

> > With git, by contrast, this won't work. Git will refuse to pull
> > anything as long as there are ANY local modifications.
> 
> Not sure what`s happening at your end, but "ANY" part shouldn`t be 
> true - you can have local modifications and still execute `git pull` 
> successfully.
> 
> Only if you have local modifications in files that _also_ changed on 
> the remote end, `git pull` aborts (fetch of the remote branch 
> succeeds, actually, just merge with local branch is aborted).

Oh, you're right! I don't know where I catched the idea that ANY modification
would stop "git pull" from working...

> Now, having in mind you said conflicts are extremely rare in your 
> flow anyway, would this be enough for you?

No. There's still the issue with "git pop", even if no modifications are
coming from upstream.

> > The cron job would need to
> > 
> >git stash
> >git pull
> >git stash pop
> > 
> > But this will temporarily remove my local modifications. If I happen
> > to do a test run at this time, the test run would NOT contain the
> > local modifications which I was about to test. Even worse: if I
> > happen to save one of the modified files while the modifications are
> > in the stash, the "git stash pop" will definitely cause a conflict,
> > although nothing really changed.
> 
> Is `git stash pop` causing conflicts your only concern here? How 
> about a situation where you save one of the modified files _after_ 
> `git stash pop` was successful, effectively discarding any updates 
> introduced by `git pull` from remote end...?

It's both. With the conflict having the highest probability.

> As you basically have a flow where two users (you and cron job) can 
> edit same files at the same time, desired outcome might be a bit 
> ambiguous, especially when scheduled execution of those files is 
> added to the mix.

Yeah. That's the difference with svn: Svn won't touch the files unless there
are changes coming from upstream. In contrast, git-stash WILL touch ALL
locally modified files, even the files which were not touched at all upstream.

So with svn, only files are at risk which are locally modified AND which have
been changed upstream. With git-stash, EVERY locally modified file is at risk.

> I`m thinking of a workflow involving (scripted) creation of a 
> temporary branch at fetched remote branch position, and using 
> something like `git checkout --merge ` to merge your 
> local modifications to latest changes fetched from remote (ending up 
> with conflicts inside working tree, if any),

But this would require local modifications to be committed?

-- 
Josef Wolf
j...@raven.inka.de

Re: What's cooking in git.git (Dec 2017, #03; Wed, 13)

[Johannes Schindelin]

Hi Junio,

On Wed, 13 Dec 2017, Junio C Hamano wrote:

> * db/doc-workflows-neuter-the-maintainer (2017-12-08) 1 commit

I hope you do not mean this literally. There is no need to bring family
planning into a technical document.

Ciao,
Dscho

Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output

[Johannes Schindelin]

Hi,

> There is a lot going on in 'run-windows-build.sh', so the output of 'set
> -x' might be useful or might be considered too much clutter, I don't
> know.  I put Dscho on Cc, I think it's mainly his call.

Certainly it might be useful.

However, please make sure that the secret token is not leaked that way.
Like, *really* sure. Due to the failure of Git to use a portable and
performant test suite, it does take about 90 minutes to build and test a
revision, therefore it would be very easy to DOS my build system, and I
really, really need it not to be DOSed because I use it in my day job, too.

Ciao,
Dscho

Re: [PATCH v2 4/4] t/Makefile: introduce TEST_SHELL_PATH

[Jeff King]

On Mon, Dec 11, 2017 at 12:37:42PM -0800, Junio C Hamano wrote:

> > Interestingly, many of those do something like this in the Makefile:
> >
> >   ifdef GIT_TEST_CMP
> > @echo GIT_TEST_OPTS=... >>$@+
> >   endif
> >
> > which seems utterly confusing to me. Because it means that if you build
> > with those options set, then they will override anything in the
> > environment. But if you don't, then you _may_ override them in the
> > environment. In other words:
> >
> >   make
> >   cd t
> >   GIT_TEST_CMP=foo ./t-*
> >
> > will respect that variable. But:
> >
> >   make GIT_TEST_CMP=foo
> >   cd t
> >   GIT_TEST_CMP=bar ./t-*
> >
> > will not. Which seems weird.  But I guess we could follow that pattern
> > with TEST_SHELL_PATH.
> 
> Or perhaps we can start setting a better example with the new
> variable, and migrate those weird existing ones over to the new way
> of not forbidding run-time overriding?

This turns out to be quite tricky, because GIT-BUILD-OPTIONS must be
parsed as both a Makefile and shell-script.

So we can do:

  1. Omit them from GIT-BUILD-OPTIONS, which means they don't work for
 cases where the tests aren't started from the Makefile (which would
 have put them in the environment). I think this is a non-starter.

  2. Add a Makefile-level ifdef when generating GIT-BUILD-OPTIONS, so
 that unused options can be overridden by the environment That's the
 "weird" thing above that we do now for some variables.

  3. Add something like:

   test -z "$FOO" && FOO=...

 when building GIT-BUILD-OPTIONS (instead of just FOO=...). But that
 doesn't work because it must parse as Makefile.

  4. In test-lib.sh, save and restore each such variable so that the
 existing environment takes precedence. Like:

   FOO_save=$FOO
   BAR_save=$BAR
   ...etc for each...

   . GIT-BUILD-OPTIONS

   test -n "$FOO_save" && FOO=$FOO_save
   test -n "$BAR_save" && BAR=$BAR_save
   ...etc...

  We have to do the save/restore dance rather than just reordering
  the assignments, because the existing environment is being passed
  into us (so we can't just "assign" it to overwrite what's in the
  build options file).

  This could be made slightly less tedious with a loop and an eval.
  It could possibly be made very succinct but very magical with
  something like:

saved=$(export)
. GIT-BUILD-OPTIONS
eval "$saved"

  5. Give up on the dual nature of GIT-BUILD-OPTIONS, and generate two
 such files (with identical values but different syntax).

I think (4) and (5) are the only things that actually change the
behavior in a meaningful way. But they're a bit more hacky and
repetitive than I'd like. Especially given that I'm not really sure
we're solving an interesting problem. I'm happy enough with the patch as
shown, and I do not recall anybody complaining about the current
behavior of these options.

> There is a long outstanding NEEDSWORK comment in help.c that wonders
> if we want to embed contents from GIT-BUILD-OPTIONS in the resulting
> binary, and the distinction Dscho brought up between "build" and
> "test" phases would start to matter even more once we go in that
> direction.

I guess you're implying having a GIT-BUILD-OPTIONS and a
GIT-TEST-OPTIONS here. But that doesn't save us from the Makefile/shell
duality, unfortunately. Some of the test options need to be read by
t/Makefile, and some need to be read by test-lib.sh (and I suspect there
are some needed in both places).

-Peff

Re: [PATCH v3] Makefile: replace perl/Makefile.PL with simple make rules

[Michael J Gruber]

Ævar Arnfjörð Bjarmason venit, vidit, dixit 12.12.2017 23:26:
> 
> On Tue, Dec 12 2017, Randall S. Becker jotted:
> 
>> -Original Message-
>> On December 10, 2017 4:14 PM, Ævar Arnfjörð Bjarmason wrote:
>> Subject: [PATCH v3] Makefile: replace perl/Makefile.PL with simple make rules
>>
>>> Replace the perl/Makefile.PL and the fallback perl/Makefile used under 
>>> NO_PERL_MAKEMAKER=NoThanks with a much simpler implementation heavily 
>>> inspired by how the i18n infrastructure's build process works[1].
>>> The reason for having the Makefile.PL in the first place is that it was 
>>> initially[2] building a perl C binding to interface with libgit, this 
>>> functionality, that was removed[3] before Git.pm ever made it to the master 
>>> branch.
>> 
>>
>> I would like to request that the we be careful that the git builds do not 
>> introduce arbitrary dependencies to CPAN. Some platforms (I can think of one 
>> off the top, being NonStop) does not provide for arbitrary additions to the 
>> supplied perl implementation as of yet. The assumption about being able to 
>> add CPAN modules may apply on some platforms but is not a general 
>> capability. I am humbly requesting that caution be used when adding 
>> dependencies. Being non-$DAYJOB responsible for the git port for NonStop, 
>> this scares me a bit, but I and my group can help validate the available 
>> modules used for builds.
>>
>> Note: we do not yet have CPAN's SCM so can't and don't use perl for access 
>> to git anyway - much that I've tried to change that.
>>
>> Please keep build dependencies to a minimum.
>>
>> Thanks for my and my whole team.
> 
> I think you should be happy with this patch then, and it doesn't add any
> more CPAN dependency than before, and sets up a framework (as discussed
> in [1]) where we can use more CPAN modules while not requiring packagers
> such as yourself to package CPAN modules.
> 
> However, it doesn't sound believable to me that even on NonStop you
> can't install any CPAN modules whatsoever.
> 
> That would also mean that this patch doesn't work for you, because it
> means that you either don't have anything resembling a hierarchical
> filesystem on which git can be installed in the first place (in which
> case it wouldn't work), or perl doesn't have an @INC to search through
> perl libs on on NonStop. What does:
> 
> perl -V
> 
> Return for you on that system?
> 
> If this patch works, and if at the bottom of `perl -V` you see some
> directories which you could write a package to drop some static *.pm
> files, then you can grab a *.tar.gz from CPAN such as the one for
> Error.pm[2] and arrange for the *.pm files contained within its lib/
> directory to be dropped into one of those @INC directories.
> 
> It may be that some aspect of the CPAN toolchain is broken for you, or
> even ExtUtils::MakeMaker, but you typically don't need that to package
> non-XS perl modules, certainly not any of the ones we've discussed
> possibly bundling up in git.git on-list recently. As a (very occasional)
> contributor to perl.git I'd be interested to know if that's what you
> mean is broken, and if so see if it could be fixed for you.
> 
> 1. 

Re: Need help migrating workflow from svn to git.

[Josef Wolf]

Thanks for your answer, Randall,

On Thu, Dec 14, 2017 at 04:07:15PM -0500, Randall S. Becker wrote:
> 
> You might want to consider a slight modification to your approach as
> follows. 
> Instead of using git pull, use git fetch.
> Have each system on its own branch (sys1 = my-sys1-branch, for example) so
> you can track who has what.
> In your scripts, consider:
> git fetch
> if nothing changed, done
> git status
> if no changes, git merge --ff  master && git push origin my-sys1-branch &&
> done
> if changes, send an email whining about the changes
> your script could then (depending on your environment) git commit -a && git
> merge && git push origin my-sys1-branch && done

The scripts never commit. In fact, they only have read access to the remote
repository. Commits are only ever done by humans manually.

So it's going to be something like this:

  git fetch origin
  if [ git diff -s master origin/master ]
git stash
git merge -ff master
git stash pop
  fi

Unfortunately, the return code of git-diff don't seem to indicate whether they
have diverged. And git-status don't seem to have an option to specify "remote
is ahead of me". How would I properly check whether a merge is actually needed?

-- 
Josef Wolf
j...@raven.inka.de

Re: Question about the ahead-behind computation and status

[Jeff King]

On Thu, Dec 14, 2017 at 04:49:31PM -0500, Jeff Hostetler wrote:

> I don't want to jump into the graph algorithm at this time,
> but was wondering about adding a --no-ahead-behind flag (or
> something similar or a config setting) that would disable
> the a/b computation during status.
> 
> For status V2 output, we could omit the "# branch.ab x y"
> line.  For normal status output, change the prose a/b
> message to say something like "are [not] up to date".

Is it actually "status --porcelain=v2" that you care about here? Or is
it normal "git status"?

Do you care about seeing the branch at all? I.e., would "--no-branch" be
a viable option (though I notice it seems to be a silent noop with the
long-form, which should probably be fixed).

If you really do want to see all branch details but just skip the
ahead/behind, then yeah, I'd agree that adding an option to do so makes
sense. Is "status" the only command that needs it? I think we do it
unconditionally with "git checkout", too.

> [*] Sadly, the local repo was only about 20 days out of
> date (including the Thanksgiving holidays)

Taking 20 seconds to traverse 20 days worth of history sounds pretty
awful. How many commits is it?

-Peff

Re: [PATCH] decorate: clean up and document API

[Jeff King]

On Mon, Dec 11, 2017 at 10:32:49AM -0800, Jonathan Tan wrote:

> > Other than that philosophical point, the documentation you added looks
> > pretty good to me. Two possible improvements to the API we could do on
> > top:
> > 
> >   1. Should there be a DECORATION_INIT macro (possibly taking the "name"
> >  as an argument)? (Actually, the whole name thing seems like a
> >  confusing and bad API design in the first place).
> 
> Agreed about the "name" thing. I'll add a DECORATION_INIT when I make
> the next reroll, but I think that having it with no argument is best
> (and instantiating "name" with NULL).

That will leave callers like the one in log-tree unable to use the
macro, since it uses a static initializer. I didn't dig, though. That
may be the only one. Most of the rest seem to just get explicitly
zero-initialized (some via xcalloc of a larger struct, so maybe we
should just promise that zero-initialization is always OK).

> >   2. This is really just an oidmap to a void pointer. I wonder if we
> >  ought to be wrapping that code (I think we still want some
> >  interface so that the caller doesn't have to declare their own
> >  structs).
> 
> It is slightly different from oidmap in that this uses "struct object *"
> as a key whereas oidmap uses "struct object_id", meaning that a user of
> "decorate" must already have objects allocated or be willing to allocate
> them, whereas a user of "oidmap" doesn't.

Ah, right. I was thinking the difference was only on the "value" half
being a pointer versus a struct.

It's nice in the current code that decorations do not incur the extra
cost of storing the oid twice (once in the "struct object", and then
again in the map key).  OTOH, that might well be premature optimization.

-Peff

Re: [PATCH v1] convert: add support for 'encoding' attribute

[Jeff King]

On Mon, Dec 11, 2017 at 04:50:23PM +0100, lars.schnei...@autodesk.com wrote:

> From: Lars Schneider 
> 
> Git and its tools (e.g. git diff) expect all text files in UTF-8
> encoding. Git will happily accept content in all other encodings, too,
> but it might not be able to process the text (e.g. viewing diffs or
> changing line endings).
> 
> Add an attribute to tell Git what encoding the user has defined for a
> given file. If the content is added to the index, then Git converts the
> content to a canonical UTF-8 representation. On checkout Git will
> reverse the conversion.

This made me wonder what happens when you have a file that _was_ utf16,
and then you later convert it to utf8 and add a .gitattributes entry.

I tried this with your patch:

  git init repo
  cd repo
  
  echo foo | iconv -t utf16 >file
  git add file
  git commit -m utf16
  
  echo 'file encoding=utf16' >.gitattributes
  touch file ;# make it stat-dirty
  git commit -m convert
  
  git checkout HEAD^

That works OK, because we try to read the attributes from the
destination tree for a checkout. If you do this:

  echo 'file encoding=utf16' >.git/info/attributes
  git checkout HEAD^

then we get:

  warning: failed to encode 'file' from utf-8 to utf16

At least it figured out that it couldn't convert the content. It's
slightly troubling that it would try in the first place, though; are
there encoding pairs where we might accidentally generate nonsense?

It _should_ be uncommon, I think, to have a repo-level attribute set
like that. It's very common for us to use whatever happens to be in the
checked-out .gitattributes for some attributes (e.g., when doing a diff
of an older commit), but I think for checkout-related ones it's not.  So
I think it may generally work in practice. And certainly the line-ending
code would share any similar problems, but at least there the result is
less confusing than mojibake.

Playing around, I also managed to do this:

  echo 'file encoding=utf16' >.gitattributes
  echo foo >file

  # I did these with an old version of git that didn't
  # support the new attribute, so they blindly added the utf8 content.
  git add .
  git commit -m convert

  git.compile checkout HEAD^

which yielded:

  fatal: encoding 'file' from utf16 to utf-8 and back is not the same

Now obviously my situation is somewhat nonsense. I was trying to force
the in-repo representation to utf8, but ended up with a mismatched
working tree file. But what's somewhat troubling is that I couldn't
checkout _away_ from that state due to the die() in convert_to_git().
Which is in turn just there as part of refresh_index().

And indeed, other commands hit the same problem:

  $ git.compile diff
  fatal: encoding 'file' from utf16 to utf-8 and back is not the same

  $ git.compile checkout -f
  fatal: encoding 'file' from utf16 to utf-8 and back is not the same

It may make sense to die() during "git add ." (since we're actually
changing the index entry, and we don't want to put nonsense into a
tree). But I'm not sure it's the best thing for operations which just
want to read the content. For them, perhaps it would be more appropriate
to issue a warning and return the untouched content.

-Peff

Assalamu`Alaikum.

[Mohammad ouattara]

Greetings from Dr. mohammad ouattara.

Assalamu`Alaikum.

My Name is Dr. mohammad ouattara, I am a banker by profession. I'm
from Ouagadougou, Burkina Faso, West Africa. My reason for contacting
you is to transfer an abandoned $14.6M to your account.

The owner of this fund died since 2004 with his Next Of Kin. I want to
present you to the bank as the Next of Kin/beneficiary of this fund.

Further details of the transaction shall be forwarded to you as soon
as I receive your return mail indicating your interest.

Have a great day,
Dr. mohammad ouattara.