Draft of Git Rev News edition 16

[Christian Couder]

Hi,

A draft of a new Git Rev News edition is available here:

  https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-16.md

Everyone is welcome to contribute in any section either by editing the
above page on GitHub and sending a pull request, or by commenting on
this GitHub issue:

  https://github.com/git/git.github.io/issues/156

You can also reply to this email.

I tried to cc everyone who appears in this edition but maybe I missed
some people, sorry about that.

Thomas, Nicola and myself plan to publish this edition on Wednesday
June 15.

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

[GIT PULL] l10n updates for 2.9.0 rc0

[Jiang Xin]

Hi Junio,

Please pull the following git l10n updates.

The following changes since commit 3a0f269e7c82aa3a87323cb7ae04ac5f129f036b:

  Git 2.9-rc0 (2016-05-23 15:02:48 -0700)

are available in the git repository at:

  git://github.com/git-l10n/git-po tags/l10n-2.9.0-rc0

for you to fetch changes up to ad583ebe0813c5d6a8e7c263d72d934770083d83:

  l10n: ko.po: Update Korean translation (2016-06-12 01:25:58 +0900)


l10n-2.9.0-rc0


Alexander Shopov (1):
  l10n: Updated Bulgarian translation of git (2597t,0f,0u)

Antonin (1):
  l10n: fr.po Fixed grammar mistake

Changwoo Ryu (1):
  l10n: ko.po: Update Korean translation

Dimitriy Ryazantcev (1):
  l10n: ru.po: update Russian translation

Jean-Noel Avila (1):
  l10n: fr.po v2.9.0rnd1

Jiang Xin (5):
  l10n: git.pot: v2.9.0 round 1 (104 new, 37 removed)
  Merge branch 'fix_fr' of git://github.com/jnavila/git
  Merge branch 'v2.9.0_rnd1_fr' of git://github.com/jnavila/git
  l10n: zh_CN: for git v2.9.0 l10n round 1
  Merge branch 'russian-l10n' of https://github.com/DJm00n/git-po-ru

Peter Krefting (1):
  l10n: sv.po: Update Swedish translation (2597t0f0u)

Ralf Thielow (1):
  l10n: de.po: translate 104 new messages

Ray Chen (1):
  l10n: zh_CN: review for git v2.9.0 l10n round 1

Trần Ngọc Quân (1):
  l10n: Updated Vietnamese translation (2597t)

Vasco Almeida (3):
  l10n: pt_PT: merge git.pot file
  l10n: pt_PT: update according to git-gui glossary
  l10n: pt_PT: update Portuguese translation

 po/bg.po| 4689 +--
 po/de.po| 3310 +++--
 po/fr.po| 3252 +++--
 po/git.pot  | 3078 ++-
 po/ko.po| 3192 ++--
 po/pt_PT.po | 3893 +++--
 po/ru.po| 3171 ++--
 po/sv.po| 3269 +++--
 po/vi.po| 3274 +++--
 po/zh_CN.po | 3375 ++
 10 files changed, 19378 insertions(+), 15125 deletions(-)

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

Re: [PATCHv4] Documentation: triangular workflow

[Philip Oakley]

From: "Ramkumar Ramachandra" 

Junio C Hamano wrote:

Jordan DE GEA wrote:

> +* Allows contributors to work with Git even though they do not have
> +write access to **UPSTREAM**.
>
> +* Allows maintainers to receive code from contributors they may not
> +trust.


Triangular workflow is the ability to accept changes from contributors
without mailing patches back-and-forth. Whether they send a pull
request or



  commit directly to the master repository


Surely, if they can do this then they do not need a distinct publish repo.

The triangle is necessary because of the accessibility 'standoff' between 
the upstream and local repos when a pull workflow is used.


Matthieu had clarified this (to me 
http://article.gmane.org/gmane.comp.version-control.git/296538) - I was (at 
that time) confusing the use of the fork as a home vault (with passive 
publishing) with the need for actively publishing a branch for a PR.



  when review is
done, is inconsequential. Essentially, they maintain forks of
upstream, which they work on at their own pace.


> +* Code review is more efficient

I have no idea what data you have to back this claim up.  More
efficient compared to what?


They're orthogonal. LLVM has one giant SVN server that everyone
commits directly to. However, they review process is a lot more
efficient than GitHub projects, because they use Phabricator. What
does code review tool have to do with triangular workflow?


> +Preparation
> +~~~
> +
> +Cloning from **PUBLISH**, which is a fork of **UPSTREAM** or an empty
> +repository.
> +
> +==
> +`git clone `
> +==
> +
> +Setting the behavior of push for the triangular workflow:
> +
> +===
> +`git config push.default current`
> +===
> +
> +Adding **UPSTREAM** remote:
> +
> +===
> +`git remote add upstream `
> +===
> +
> +With the `remote add` above, using `git pull upstream` pulls there,
> +instead of saying its URL. In addition, `git pull` can pull from
> +**UPSTREAM** without argument.
> +
> +For each branch requiring a triangular workflow, set
> +`branch..remote` and `branch..pushRemote`.
> +
> +Example with master as :
> +===
> +* `git config branch.master.remote upstream`
> +* `git config branch.master.pushRemote origin`
> +===


It's much too simple now. Just `git clone `, `git remote add
mine `, and `git config remote.pushdefault mine`. Only the
last line requires an explanation.


I note that you use 'mine', Jordan was proposing 'me', while I started using 
'my'. It is useful to see these personal choices, especially as there is no 
'origin' in the nominal triangular flow diagram.


Whether to use branch configs or remote configs is part of the 
clarifications.





Instead you would set default.pushRemote to publish
just once, and no matter how many branches you create later, you do
not have to do anything special.


I think you meant remote.pushdefault here?



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

Re: [PATCHv4] Documentation: triangular workflow

[Philip Oakley]

From: "Junio C Hamano" 

"Philip Oakley"  writes:


+Preparation
+~~~
+
+Cloning from **PUBLISH**, which is a fork of **UPSTREAM** or an empty
+repository.


I agree here. To clone the upstream, to which you have no push access (by
definition), would leave the config badly mis-set for the basic user.
It's
better for the user to clone their publish fork (to which they have both
read and write access).


I do not think I agree.

If you apriori know that you do want to hack on a project's code, then
forking at GitHub first and then cloning the copy would be OK.



You've clipped my other point:

-One issue may be the different expectations of how the fork is
-created (it's only one click on the GitHub..)

The fork creation issue is surely an essential part of triangular workflow - 
a fork isn't required for a patch workflow. Git uses patch flow, while 
Git-for-Windows makes use of (GitHub's) Pull Requests.


Most projects will be using some form of hosting service to provide their 
public publish repo, and often allow no public pushes. It's the upstream 
that choses the hosting service, which decides the users choices.



But I doubt that would be a common set-up, unless you are focusing
only on school-like setting where you are told by your instructor to
"make changes to this public project, and show the result in your
fork".  In real life you cannot tell if the project is worth your
time modifying until you see it first, can you?


At this point there are multiple choices depending how the hosting and 
project is set up. It may already be packaged. It may allow full web 
browsing of the code. It probably has tar/zip download of the latest master. 
Some popular sites offer free hosting and forking, which leads to the the 
suggested method.




I suspect that the majority of local clones start from something
like "I want to build and use from the tip", "I want to use a module
that does X, and there are three candidates, so let's clone them all
to evaluate", etc.  You do not bother "forking at GitHub" but just
clone from the upstream for these clones.


Any stat's for this. I'd be far more likely to fork first, giving me a 
backup vault, and clone that, especially since being bitten (historically) 
by the need to juggle the configs after the fact..




After you build it and try things out, you may start making local
changes, and you may even record your changes as local commits.  You
play with your local clone of the upstream.  After doing so, you may
find that some of the projects do not fit your needs, but for some
others, you would find that it is worth your time and effort to
upstream your changes and/or keep working further on the project.


In all these cases there is the 'backup' copy question for any of those 
mods, which tends to mean the user will have a fork acting as a home vault.




And at that point, you would create a publishing place, push into
it, and tell others "Hey I did this interesting thing!".  That
"creat a publishing place" step could be just a one click at GitHub.

Isn't that how you work with other people's projects?  Or do you
always modify every project you fetch from the outside world?, Do
you always fork first, just in case you *might* change and you
*might* have to have a place to push your changes out?


As noted above, yes, if I'm interested enough to go for the clone, then mods 
are a real posibility and I'd typically use the hosting service as a 
vault/triangle.




If you tell novices "You fork first and then clone your fork", and
in the ideal (to you) case they will follow that advice to the
letter and they will end up with forks of all projects they will
ever look at, in many of which they make no local commit.

What is more likely to happen is that they will first ignore you and
start from a local clone of the upstream, and then find this
document that says "triangular workflow requires you to fork first,
clone that fork and work in it".  Because they would have to fork
first and make another clone, this time a clone of the fork, in
order to follow the instruction of this document, they oblige,
ending up with two clones.  More importantly, this makes the local
clone of the upstream they made earlier and the changes they made in
that clone appear useless.  They need to be told how to transplant
the work done in the clone to the newly created clone of the fork,
in order to publish them.

If your instruction begins with "You clone from upstream as usual
(i.e. just like when you make a "read-only" clone without any
intention to make changes or push changes out), and add a publish
place if/when it becomes necessary", the problem described in the
previous paragraph goes away, no?



Yes, the document does need to cover both routes, which should be the main 
outcome of the discussion.


But I still think that, by definition of the triangular workflow (and the 
reader will be here because they are expecting to 

Re: [PATCHv4] Documentation: triangular workflow

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> Jordan DE GEA wrote:
>
> > +* Allows contributors to work with Git even though they do not have
> > +write access to **UPSTREAM**.
> >
> > +* Allows maintainers to receive code from contributors they may not
> > +trust.

Triangular workflow is the ability to accept changes from contributors
without mailing patches back-and-forth. Whether they send a pull
request or commit directly to the master repository when review is
done, is inconsequential. Essentially, they maintain forks of
upstream, which they work on at their own pace.

> > +* Code review is more efficient
>
> I have no idea what data you have to back this claim up.  More
> efficient compared to what?

They're orthogonal. LLVM has one giant SVN server that everyone
commits directly to. However, they review process is a lot more
efficient than GitHub projects, because they use Phabricator. What
does code review tool have to do with triangular workflow?

> > +Preparation
> > +~~~
> > +
> > +Cloning from **PUBLISH**, which is a fork of **UPSTREAM** or an empty
> > +repository.
> > +
> > +==
> > +`git clone `
> > +==
> > +
> > +Setting the behavior of push for the triangular workflow:
> > +
> > +===
> > +`git config push.default current`
> > +===
> > +
> > +Adding **UPSTREAM** remote:
> > +
> > +===
> > +`git remote add upstream `
> > +===
> > +
> > +With the `remote add` above, using `git pull upstream` pulls there,
> > +instead of saying its URL. In addition, `git pull` can pull from
> > +**UPSTREAM** without argument.
> > +
> > +For each branch requiring a triangular workflow, set
> > +`branch..remote` and `branch..pushRemote`.
> > +
> > +Example with master as :
> > +===
> > +* `git config branch.master.remote upstream`
> > +* `git config branch.master.pushRemote origin`
> > +===

It's much too simple now. Just `git clone `, `git remote add
mine `, and `git config remote.pushdefault mine`. Only the
last line requires an explanation.

> Instead you would set default.pushRemote to publish
> just once, and no matter how many branches you create later, you do
> not have to do anything special.

I think you meant remote.pushdefault here?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Gitk Inotify support

[Florian Schüller]

>From 74d2f4c1ec560b358fb50b8b7fe8282e7e1457b0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20Sch=C3=BCller?= 
Date: Thu, 9 Jun 2016 22:54:43 +0200
Subject: [PATCH] first support for inotify
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Just automatically update gitk when working in a terminal on the same repo

Open points for now:
 - release watches for deleted directories seems to
   cause problems in tcl-inotify (so I don't)
   I'm not sure how often that happens in ".git/"
 - I only call "updatecommits" and I don't know if there is a usecase
   where I should be calling "reloadcommits"

Signed-off-by: Florian Schüller 
---
 gitk | 53 +
 1 file changed, 53 insertions(+)

diff --git a/gitk b/gitk
index 805a1c7..58e3dca 100755
--- a/gitk
+++ b/gitk
@@ -8,6 +8,12 @@ exec wish "$0" -- "$@"
 # either version 2, or (at your option) any later version.

 package require Tk
+try {
+package require inotify
+set have_inotify true
+} on error {em} {
+set have_inotify false
+}

 proc hasworktree {} {
 return [expr {[exec git rev-parse --is-bare-repository] == "false" &&
@@ -12363,6 +12369,53 @@ if {$i >= [llength $argv] && $revtreeargs ne {}} {
 }
 }

+proc inotify_handler { fd } {
+set events [inotify_watch read]
+set watch_info [inotify_watch info]
+set update_view false
+
+foreach {event} $events {
+set current_watchid [dict get $event watchid]
+set flags [dict get $event flags]
+set event_filename [dict get $event filename]
+
+foreach {path watchid watch_flags} $watch_info {
+if {$watchid eq $current_watchid} {
+set watch_path $path
+}
+}
+
+set full_filename [file join $watch_path $event_filename]
+
+if {$flags eq "nD"} {
+set wd [inotify_watch add $full_filename "nwds"]
+}
+if {![string match *.lock $event_filename]} {
+set update_view true
+}
+}
+
+#reloadcommits or updatecommits - depending on file and operation?
+if {$update_view} {
+updatecommits
+}
+}
+
+proc watch_recursive { dir } {
+inotify_watch add $dir "nwaCmMds"
+
+foreach i [glob -nocomplain -dir $dir *] {
+if {[file type $i] eq {directory}} {
+watch_recursive $i
+}
+}
+}
+
+if { $have_inotify } {
+set fd [inotify create "inotify_watch" "::inotify_handler"]
+watch_recursive $gitdir
+}
+
 set nullid ""
 set nullid2 "0001"
 set nullfile "/dev/null"
-- 
2.7.4

On Thu, Jun 9, 2016 at 11:24 PM, Stefan Beller  wrote:
> On Thu, Jun 9, 2016 at 2:12 PM, Florian Schüller
>  wrote:
>> Hi
>> Is this correct to send possible gitk patches here? or should I send
>> them to Paul Mackerras somehow?
>
> I cc'd Paul for you :)
>
>>
>> Anyway I just wanted that gitk automatically updates while I'm working
>> in my terminal
>
> Thanks for coming up with a patch. Welcome to the Git community!
>
>>
>> Are you interrested?
>>
>
> Also see the section that talks about signing off the patch and how to
> send the patch
> inline. :)
>
>
>> From 785ed6bc1b4a3b9019d3503b066afb2a025a2bc1 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Florian=20Sch=C3=BCller?= 
>> Date: Thu, 9 Jun 2016 22:54:43 +0200
>> Subject: [PATCH] first support for inotify
>
> Here you should describe your change, i.e. what problem is solved in this 
> patch,
> what are the alternatives, why is this way the best? Also the sign off
> goes here.
>
>>
> ---
>>  gitk | 59 +++
>>  1 file changed, 59 insertions(+)
>>
> diff --git a/gitk b/gitk
>> index 805a1c7..6e2ead2 100755
>> --- a/gitk
>> +++ b/gitk
>> @@ -8,6 +8,12 @@ exec wish "$0" -- "$@"
>>  # either version 2, or (at your option) any later version.
>>
>>  package require Tk
>> +try {
>> +package require inotify
>> +set we_have_inotify true
>> +} on error {em} {
>> +set we_have_inotify false
>> +}
>
> There are quite a few "have_*" variables, so I would drop the leading "we_"
>
>>
>>  proc hasworktree {} {
>>  return [expr {[exec git rev-parse --is-bare-repository] == "false" &&
>> @@ -12363,6 +12369,59 @@ if {$i >= [llength $argv] && $revtreeargs ne {}} {
>>  }
>>  }
>>
>> +proc inotify_handler { fd } {
>> +set events [inotify_watch read]
>> +set watch_info [inotify_watch info]
>> +set update_view false
>> +
>> +foreach {event} $events {
>> +set current_watchid [dict get $event watchid]
>> +set flags [dict get $event flags]
>> +set event_filename [dict get $event filename]
>> +
>> +foreach {path watchid watch_flags} $watch_info {
>> +if {$watchid eq $current_watchid} 

Re: [PATCH 1/2] bisect--helper: `is_expected_rev` shell function in C

[Pranit Bauva]

Hey Eric,

On Sat, Jun 11, 2016 at 12:44 AM, Eric Sunshine  wrote:
> On Fri, Jun 10, 2016 at 9:39 AM, Pranit Bauva  wrote:
>> On Fri, Jun 10, 2016 at 3:03 AM, Eric Sunshine  
>> wrote:
>>> On Wed, Jun 8, 2016 at 11:24 AM, Pranit Bauva  
>>> wrote:
 Reimplement `is_expected_rev` shell function in C. This will further be
 called from `check_expected_revs` function. This is a quite small
 function thus subcommand facility is redundant.
>>>
>>> This patch should be squashed into patch 2/2, as it is otherwise
>>> pointless without that patch, and merely adds dead code.
>>
>> Sure I will squash and will explain it in the commit message.
>
> Explain what in the commit message? If anything, I'd expect the commit
> message to shrink since you won't need to explain anymore that this
> function is split out.

Yes I would remove the part where it is explained that this function
is split out. I will just explain that 2 functions are converted in 1
commit.

 +   if (!file_exists(git_path_bisect_expected_rev()))
 +   return 0;
>>>
>>> Invoking file_exists() seems unnecessarily redundant when you can
>>> discern effectively the same by checking the return value of
>>> strbuf_read_file() below. I'd drop the file_exists() check altogether.
>>
>> I wanted to imitate the code. But I guess it would actually be better
>> if I drop this file_exists().
>
> There is a bit of a lesson to be learned by this example. While it's
> true that the C conversion should retain the behavior of the original
> shell code, that does not mean blindly mirroring the implementation
> line for line is a good idea. A couple things to take into
> consideration:
>
> There are idiomatic ways of doing things in each language. What is
> idiomatic in shell is not necessarily so in C. The C conversion should
> employ C idioms and flow in a way which is natural for C code.
>
> Consider what the original shell code is doing at a higher level than
> merely by reading it line-by-line. In the case in question, the code
> is:
>
> test -f "$GIT_DIR/BISECT_EXPECTED_REV" &&
> test "$1" = $(cat "$GIT_DIR/BISECT_EXPECTED_REV")
>
> While it's true that it's asking "does the file exist and is its value
> the same as $1", the 'test -f' avoids a "file not found" error from
> the $(cat ...) invocation. Since the return value of
> strbuf_read_file() effectively encapsulates the "does the file exist"
> check, a separate check isn't really needed.

True. I will keep this in mind.

 +   if (!strbuf_read_file(_hex, git_path_bisect_expected_rev(), 
 0))
 +   return 0;
>>>
>>> What exactly is this trying to do? Considering that strbuf_read_file()
>>> returns -1 upon error, otherwise the number of bytes read, if I'm
>>> reading this correctly, is_expected_rev() returns false if
>>> strbuf_read_file() encounters an error (which is fine) but also when
>>> it successfully reads the file and its content length is non-zero
>>> (which is very odd).
>>>
 +   strbuf_trim(_hex);
 +   return !strcmp(actual_hex.buf, expected_hex);
>>>
>>> Thus, it only ever gets to this point if the file exists but is empty,
>>> which is very unlikely to match 'expected_hex'. I could understand it
>>> if you checked the result of strbuf_read_file() with <0 or even <=0,
>>> but the current code doesn't make sense to me.
>>>
>>> Am I misunderstanding?
>>
>> Definitely not. Thanks for pointing it out. :) It went off my head
>> that strbuf_read_file returns the bytes it reads. Also the code
>> comment regarding strbuf_read_file does not mention it which probably
>> misguided me. I should also send a fixing patch so that someone else
>> does not fall into this like I did.
>
> Out of curiosity, did the test suite pass with this patch applied?
> This is such an egregious bug that it's hard to imagine the tests
> passing, but if they did, then that may be a good indication that
> coverage is too sparse and ought to be improved.

Yes the test suite passed perfectly. I have inculcated the habit of
running the whole test suite before sending patches. Yes some parts of
a test suite seem to be missing. How about I do it in the end? By this
I won't have to setup yet another coverage tool for shell script. I
can use the coverage tool by GNU to test the coverage after bisect is
a C code. Till that time the patches can reside in the pu branch.

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

Re: feature request: git svn dommit --preserve-timestamps

[Eric Wong]

Peter Münster  wrote:
> On Sat, Jun 11 2016, Eric Wong wrote:
> 
> > The git log after dcommit is tied to the SVN log,
> > so git-svn can only reflect changes which appear in SVN.
> 
> You mean, it's impossible, to keep the original timestamps??

It might be; just not easy; and I haven't looked at the code
in ages.  But there seems to be similar options for preserving
authorship in git-only  (see below)

> > Unfortunately, you would have to care about svn log as long as
> > SVN exists in your workflow and you need to interact with SVN
> > users.
> 
> In my case, all development happens on Git, SVN is only some sort of
> copy. And when the original timestamps are lost, I've sometimes some
> real problems in finding a specific commit that matches another event.

I'm sorry for your situation and hoping you migrate off SVN
entirely, soon :)

> > git svn tries hard to work transparently and as close to the
> > behavior of the upstream SVN repo as possible.
> 
> That's why I suggest an option, for use cases as mine. Those, who prefer
> to keep the current behaviour just won't use it.
> 
> If someone could guide me through the code, I could modify it perhaps.

Maybe you could look at how the _use_log_author and
_add_author_from options work.  I've forgotten their existence
until now and I've never used them myself; but apparently
they're still there.

Unfortunately, if you have other users using git-svn;
it could be tricky to ensure they can see the same timestamps
you see...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 31/44] run-command: make dup_devnull() non static

[Christian Couder]

On Sat, Jun 11, 2016 at 10:17 AM, Johannes Sixt  wrote:
> Am 10.06.2016 um 22:11 schrieb Christian Couder:
>>
>> --- a/run-command.c
>> +++ b/run-command.c
>> @@ -85,7 +85,7 @@ static inline void close_pair(int fd[2])
>>   }
>>
>>   #ifndef GIT_WINDOWS_NATIVE
>> -static inline void dup_devnull(int to)
>> +void dup_devnull(int to)
>
>
> I'm not adding arguments to what we've heard on whether to use /dev/null in
> this series or not. But if the outcome is to keep this patch, please remove
> the #ifndef that we see in the context lines (and the matching #endif), too.
> Otherwise, the build fails on Windows for each patch in the series until
> this change is reverted in patch 42/44.

Ok, I will find a way to avoid that build failure, though I don't test
on Windows.

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

Re: [PATCH v6 43/44] builtin/apply: add a cli option for be_silent

[Christian Couder]

On Fri, Jun 10, 2016 at 10:59 PM, René Scharfe  wrote:
> Am 10.06.2016 um 22:11 schrieb Christian Couder:
>>
>> Let's make it possible to request a silent operation on the
>> command line.
>>
>> Signed-off-by: Christian Couder 
>> ---
>>   builtin/apply.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> index ddd61de..93744f8 100644
>> --- a/builtin/apply.c
>> +++ b/builtin/apply.c
>> @@ -74,6 +74,8 @@ int cmd_apply(int argc, const char **argv, const char
>> *prefix)
>> OPT_BOOL(0, "allow-overlap", _overlap,
>> N_("allow overlapping hunks")),
>> OPT__VERBOSE(_verbosely, N_("be verbose")),
>> +   OPT_BOOL(0, "silent", _silent,
>> +   N_("do not print any output")),
>> OPT_BIT(0, "inaccurate-eof", ,
>> N_("tolerate incorrectly detected missing new-line
>> at the end of file"),
>> APPLY_OPT_INACCURATE_EOF),
>
> Why not -q/--quiet as for most other commands?

First as I say in the cover letter, I am going to discard this patch.
That is because it is not necessary, and it appeared a bit
controversial whether I should use -q/--quiet or not in the v2
discussions.

> Furthermore, you could use OPT__VERBOSITY, which causes -v and -q to update
> the same variable variable and thus lets parseopt handle their interaction.
> Perhaps verbosity == 1 could mean verbose, 0 normal, -1 no infos, -2 no
> warnings and -3 no errors?

Yeah, that could be done.

> And if you add the ability to silence the apply functions before using them
> you don't have to export and unexport dup_devnull().

Yeah, I will do something like that in the next version.

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

Re: [PATCH v6 41/44] am: use be_silent in 'struct apply_state' to shut up applying patches

[Christian Couder]

On Sat, Jun 11, 2016 at 12:07 AM, Junio C Hamano  wrote:
> The update in 33/44 to make am call into apply that is not ready to
> be called (e.g. the caller needs the dup(2) dance with /dev/null to
> be silent) gets finally corrected with this step, which makes the
> progress of the topic somewhat ugly and reviewing it a bit harder
> than necessary.  As it stands, the last several patches in the
> series smells more like "oops, we realize these things were not done
> properly the first time, and here are the follow-up patches to fix
> them up".
>
> I wonder if it is a good idea to delay integrating the apply
> machinery into "am" until it is ready?

Ok, I will do that and I think it should also avoid the build failures
on Windows.

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

Re: [PATCH v6 31/44] run-command: make dup_devnull() non static

[Johannes Sixt]

Am 10.06.2016 um 22:11 schrieb Christian Couder:

--- a/run-command.c
+++ b/run-command.c
@@ -85,7 +85,7 @@ static inline void close_pair(int fd[2])
  }

  #ifndef GIT_WINDOWS_NATIVE
-static inline void dup_devnull(int to)
+void dup_devnull(int to)


I'm not adding arguments to what we've heard on whether to use /dev/null 
in this series or not. But if the outcome is to keep this patch, please 
remove the #ifndef that we see in the context lines (and the matching 
#endif), too. Otherwise, the build fails on Windows for each patch in 
the series until this change is reverted in patch 42/44.


-- Hannes

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

Re: [PATCH v2 00/94] libify apply and use lib in am

[Johannes Schindelin]

Hi Hannes,

On Fri, 10 Jun 2016, Johannes Sixt wrote:

> Am 10.06.2016 um 13:11 schrieb Johannes Schindelin:
> > On Fri, 10 Jun 2016, Christian Couder wrote:
> >
> > > I fixed this by moving the "close(fd)" call just after the
> > > "apply_patch()" call.
> 
> This bug in v1 was discovered by the test suite and fixed in v2.

Maybe it would be a good idea to fix test failures before sending out an
80-strong patch series.

Just sayin'.

> > and this:
> >
> > > I will have another look at the 2 other places where there are
> > > open()/close() or fopen()/fclose() calls.
> >
> > but nothing about a careful, systematic investigation of all error code
> > paths. As a consequence, I fully expect to encounter test failures as soon
> > as I test your patch series again, simply because resources are still in
> > use when they should no longer be used. In other words, my expectations
> > are now lower than they have been before, my concerns are not at all
> > addressed.
> 
> Do you trust the test suite to some degree?

Yes. To *some* degree.

For comparison, my rebase--helper patches pass the test suite for more
than a month now. I still discovered two minor problems and fixed them
yesterday.

Our test suite (and *any* test suite, really) is in *no way* a substitute
for proper review. And the first review needs to be performed by the
developer herself.

Just compare the two options: to add tests for each and every corner case,
especially error code paths, or alternatively just going through the patch
manually, inspecting every error code path from the used-resource point of
view?

I am sure you agree that the latter is a much better use of everybody's
time, and also that that review will be most effective when performed by
the person most familiar with the patches.

> It passes after the above bug was fixed in v2. In addition, haven't
> found any problems so far during daily use.

I put much more stock into the latter than the former. The former is
required, to be sure, but by far not sufficient.

TBH I was quite disappointed when I tried to run v1 and found such a
simple bug right away, which made me wonder how many more not-so-simple
bugs were to be found. All that, while I really wanted this patch series
to be good enough to enter core Git's code base.

> > > This is the newest iteration:
> > >
> > > https://github.com/chriscool/git/commits/libify-apply-use-in-am65
> >
> > And that cute 65 in the name is the revision.
> 
> Yeah, that number is painful. I would appreciate an unversioned branch
> name, too.

To be frank, I think that a version number in a branch name is incorrect
Git usage. Version numbers are something for tags, not for branches (I
would understand partial version numbers in maintenance branches, of
course, because then they would not version the *branch* but convey the
purpose of the branch, as names should).

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

Re: feature request: git svn dommit --preserve-timestamps

[Peter Münster]

On Sat, Jun 11 2016, Eric Wong wrote:

> The git log after dcommit is tied to the SVN log,
> so git-svn can only reflect changes which appear in SVN.

You mean, it's impossible, to keep the original timestamps??


>   Sidenote: The convention is reply-to-all on lists like
>   this one which do not require subscription to post.

Ok, thanks.


> Unfortunately, you would have to care about svn log as long as
> SVN exists in your workflow and you need to interact with SVN
> users.

In my case, all development happens on Git, SVN is only some sort of
copy. And when the original timestamps are lost, I've sometimes some
real problems in finding a specific commit that matches another event.


> git svn tries hard to work transparently and as close to the
> behavior of the upstream SVN repo as possible.

That's why I suggest an option, for use cases as mine. Those, who prefer
to keep the current behaviour just won't use it.

If someone could guide me through the code, I could modify it perhaps.

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