Re: What's cooking in git.git (Jun 2017, #04; Tue, 13)
Jonathan Nieder writes: >> It is not known if a simple "yes/no" is sufficient in the longer >> term, and what should happen when --recurse-submodules option starts >> taking "recurse into them how?" parameter, though. > > Any pointers for where this has been discussed, if anywhere (e.g. was If this were discussed, then the answer to the question we may know by now ;-) I do not think anybody gave a serious thought to convince the public why a boolean is enough, hence this comment. >> * bw/config-h (2017-06-13) 4 commits >> - config: don't implicitly use gitdir >> - config: don't include config.h by default >> - config: remove git_config_iter >> - config: create config.h >> >> Code clean-up. > > Patches 1-3 are good to go IMHO. > > Patch 4 in pu is marked with my Reviewed-by. I think it's getting > there but not there yet. Did some script pull the tag from my reply > to the cover letter? No, nothing that elaborate. I go through each message in Gnus newsreader and feed the article to a shell command, e.g. "Meta/add-by -r jrnieder@ | git am -s3c". The UI remembers the last command I used when I choose to feed the next article to a shell command, and after running it to first three, I forgot to remove the 'add-by' bit from the command line for the fourth one.
Re: rs/strbuf-addftime-zZ, was Re: What's cooking in git.git (Jun 2017, #04; Tue, 13)
Am 15.06.2017 um 07:42 schrieb Jeff King: On Thu, Jun 15, 2017 at 01:03:29AM +0200, René Scharfe wrote: But there's more. strftime on Windows doesn't support common POSIX- defined tokens like %F (%Y-%m-%d) and %T (%H:%M:%S). We could handle them as well. Do we want that? At least we'd have to update the added test that uses them.. Here's the full list of tokens in POSIX [1], but not supported by Windows [2]: %C, %D, %F, %G, %R, %T, %V, %e, %g, %h, %n, %r, %t, %u plus the modifiers %E and %O. I don't have a real opinion on that. The point of adding strftime was always to give the user access to whatever their system supports. In particular "%c" which we cannot emulate ourselves. If people want support for those other things on platforms that don't have it, I have no real objection. But I also don't know that it's worth spending time on if nobody is asking for it. Agreed; let's make the tests more focused (i.e. not exercise %F and %T needlessly). René
Re: rs/strbuf-addftime-zZ, was Re: What's cooking in git.git (Jun 2017, #04; Tue, 13)
On Thu, Jun 15, 2017 at 01:03:29AM +0200, René Scharfe wrote: > Am 14.06.2017 um 23:04 schrieb Johannes Schindelin: > > On Wed, 14 Jun 2017, René Scharfe wrote: > > > >> Does someone actually expect %z to show time zone names instead of > >> offsets on Windows? > > > > Not me ;-) > > > > I cannot speak for anyone else, as I lack that information, though. > > Before the patch %z would always expand to + on Linux and to the > name of the local time zone on Windows, no matter which offset was > actually given. So it was broken in either case (even though it got > at least some aspects right by accident for some commits). Based on > that I'd think handling %z internally should be OK. I agree. > But there's more. strftime on Windows doesn't support common POSIX- > defined tokens like %F (%Y-%m-%d) and %T (%H:%M:%S). We could handle > them as well. Do we want that? At least we'd have to update the > added test that uses them.. > > Here's the full list of tokens in POSIX [1], but not supported by > Windows [2]: %C, %D, %F, %G, %R, %T, %V, %e, %g, %h, %n, %r, %t, %u > plus the modifiers %E and %O. I don't have a real opinion on that. The point of adding strftime was always to give the user access to whatever their system supports. In particular "%c" which we cannot emulate ourselves. If people want support for those other things on platforms that don't have it, I have no real objection. But I also don't know that it's worth spending time on if nobody is asking for it. -Peff
Re: rs/strbuf-addftime-zZ, was Re: What's cooking in git.git (Jun 2017, #04; Tue, 13)
Am 14.06.2017 um 23:04 schrieb Johannes Schindelin: > On Wed, 14 Jun 2017, René Scharfe wrote: > >> Does someone actually expect %z to show time zone names instead of >> offsets on Windows? > > Not me ;-) > > I cannot speak for anyone else, as I lack that information, though. Before the patch %z would always expand to + on Linux and to the name of the local time zone on Windows, no matter which offset was actually given. So it was broken in either case (even though it got at least some aspects right by accident for some commits). Based on that I'd think handling %z internally should be OK. But there's more. strftime on Windows doesn't support common POSIX- defined tokens like %F (%Y-%m-%d) and %T (%H:%M:%S). We could handle them as well. Do we want that? At least we'd have to update the added test that uses them.. Here's the full list of tokens in POSIX [1], but not supported by Windows [2]: %C, %D, %F, %G, %R, %T, %V, %e, %g, %h, %n, %r, %t, %u plus the modifiers %E and %O. René [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/strftime.html [2] https://msdn.microsoft.com/en-us/library/fe06s4ak.aspx
Re: rs/strbuf-addftime-zZ, was Re: What's cooking in git.git (Jun 2017, #04; Tue, 13)
Hi, On Wed, 14 Jun 2017, René Scharfe wrote: > Does someone actually expect %z to show time zone names instead of > offsets on Windows? Not me ;-) I cannot speak for anyone else, as I lack that information, though. Ciao, Dscho
Re: rs/strbuf-addftime-zZ, was Re: What's cooking in git.git (Jun 2017, #04; Tue, 13)
Am 14.06.2017 um 13:10 schrieb Jeff King: On Wed, Jun 14, 2017 at 12:57:06PM +0200, Johannes Schindelin wrote: But even then, it fails in t0006 on Windows with this error: -- snip -- ++ eval 'diff -u "$@" ' +++ diff -u expect actual --- expect 2017-06-14 10:53:40.126136900 + +++ actual 2017-06-14 10:53:40.171146800 + @@ -1 +1 @@ -146600 +0200 -> 2016-06-15 14:13:20 + (UTC) +146600 +0200 -> 2016-06-15 14:13:20 UTC (UTC) Ugh, I was worried about that some systems might display timezones differently (that's why I _didn't_ check %Z in the EST5 case). But I must admit this was not an incompatibility I was expecting. It looks like your system strftime() turns %z into "UTC". POSIX says: %z Replaced by the offset from UTC in the ISO 8601:2000 standard format (+hhmm or -hhmm), or by no characters if no timezone is determinable. So it seems like the mingw strftime is violating POSIX. I don't see an easy solution beyond marking this as !MINGW. Though if we wanted a partial test, we could test %z and %Z separately. Hmm. The patches currently either let strftime handle both %z and %Z (in the local case) or handle both internally. Perhaps we need a third option, namely to handle %z internally in all cases for systems whose implementation violates POSIX? Nah, it would be easier to always handle %z internally. Any downsides? Does someone actually expect %z to show time zone names instead of offsets on Windows? René
Re: What's cooking in git.git (Jun 2017, #04; Tue, 13)
On 14.06.17 09:42, Lars Schneider wrote: > >> * ls/filter-process-delayed (2017-06-01) 5 commits >> - convert: add "status=delayed" to filter process protocol >> - convert: move multiple file filter error handling to separate function >> - t0021: write "OUT" only on success >> - t0021: make debug log file name configurable >> - t0021: keep filter log files on comparison >> >> The filter-process interface learned to allow a process with long >> latency give a "delayed" response. >> >> Needs review. > > I wonder if anyone has a few free cycles to review this: > http://public-inbox.org/git/20170601082203.50397-1-larsxschnei...@gmail.com/ It's on my todo-list, may be this weekend or so
Re: rs/strbuf-addftime-zZ, was Re: What's cooking in git.git (Jun 2017, #04; Tue, 13)
Johannes Schindelin writes:
> Hold on. Have you tried to build this branch?
>
> -- snip --
> CC date.o
> date.c:63:36: error: unknown type name ‘timestamp_t’
> static struct tm *time_to_tm_local(timestamp_t time)
> ^
> date.c: In function ‘show_date’:
> date.c:211:8: error: implicit declaration of function ‘time_to_tm_local’
> [-Werror=implicit-function-declaration]
>tm = time_to_tm_local(time);
> ^
> date.c:211:6: error: assignment makes pointer from integer without a cast
> [-Werror=int-conversion]
>tm = time_to_tm_local(time);
> ^
> cc1: all warnings being treated as errors
> -- snap --
>
> I would expect this to be rebased *at least* to dddbad728c9 (timestamp_t:
> a new data type for timestamps, 2017-04-26).
Thanks for noticing.
My preference is to use ulong in the commit at the tip by Peff. We
can do s/ulong/timestamp_t/ in a merge that merges the topic to
newer integration branches that have the timestamp_t topic merged
(e.g. 'master'), as that allows older integration branches
(e.g. 'maint') to have the %z/%Z fix independently.
It would be another valid approach to fork it at b15667bb ("Merge
branch 'js/larger-timestamps'", 2017-05-16) or a commit that appears
later than that one on 'master', and tweak Peff's commit to use
timestamp_t, if we declare that the %z/%Z fix will only be in 2.14
and later and will never go to 'maint'.
Re: rs/strbuf-addftime-zZ, was Re: What's cooking in git.git (Jun 2017, #04; Tue, 13)
On Wed, Jun 14, 2017 at 12:57:06PM +0200, Johannes Schindelin wrote: > Hi Junio, > > On Tue, 13 Jun 2017, Junio C Hamano wrote: > > > * rs/strbuf-addftime-zZ (2017-06-10) 2 commits > > - date: use localtime() for "-local" time formats > > - strbuf: let strbuf_addftime handle %z and %Z itself > > > > As there is no portable way to pass timezone information to > > strftime, some output format from "git log" and friends are > > impossible to produce. Teach our own strbuf_addftime to replace %z > > and %Z with caller-supplied values to help working around this. > > > > Will merge to 'next'. > > Hold on. Have you tried to build this branch? > > -- snip -- > CC date.o > date.c:63:36: error: unknown type name ‘timestamp_t’ > static struct tm *time_to_tm_local(timestamp_t time) > ^ > date.c: In function ‘show_date’: > date.c:211:8: error: implicit declaration of function ‘time_to_tm_local’ > [-Werror=implicit-function-declaration] >tm = time_to_tm_local(time); > ^ > date.c:211:6: error: assignment makes pointer from integer without a cast > [-Werror=int-conversion] >tm = time_to_tm_local(time); > ^ > cc1: all warnings being treated as errors > -- snap -- > > I would expect this to be rebased *at least* to dddbad728c9 (timestamp_t: > a new data type for timestamps, 2017-04-26). Yeah, the timestamp_t mentions are from my patch (the top one). I built it applying René's on the current master and then building on top. I suspect Junio didn't test it in isolation, but only merged to "pu", where it would be OK. > But even then, it fails in t0006 on Windows with this error: > > -- snip -- > ++ eval 'diff -u "$@" ' > +++ diff -u expect actual > --- expect 2017-06-14 10:53:40.126136900 + > +++ actual 2017-06-14 10:53:40.171146800 + > @@ -1 +1 @@ > -146600 +0200 -> 2016-06-15 14:13:20 + (UTC) > +146600 +0200 -> 2016-06-15 14:13:20 UTC (UTC) Ugh, I was worried about that some systems might display timezones differently (that's why I _didn't_ check %Z in the EST5 case). But I must admit this was not an incompatibility I was expecting. It looks like your system strftime() turns %z into "UTC". POSIX says: %z Replaced by the offset from UTC in the ISO 8601:2000 standard format (+hhmm or -hhmm), or by no characters if no timezone is determinable. So it seems like the mingw strftime is violating POSIX. I don't see an easy solution beyond marking this as !MINGW. Though if we wanted a partial test, we could test %z and %Z separately. -Peff
rs/strbuf-addftime-zZ, was Re: What's cooking in git.git (Jun 2017, #04; Tue, 13)
Hi Junio, On Tue, 13 Jun 2017, Junio C Hamano wrote: > * rs/strbuf-addftime-zZ (2017-06-10) 2 commits > - date: use localtime() for "-local" time formats > - strbuf: let strbuf_addftime handle %z and %Z itself > > As there is no portable way to pass timezone information to > strftime, some output format from "git log" and friends are > impossible to produce. Teach our own strbuf_addftime to replace %z > and %Z with caller-supplied values to help working around this. > > Will merge to 'next'. Hold on. Have you tried to build this branch? -- snip -- CC date.o date.c:63:36: error: unknown type name ‘timestamp_t’ static struct tm *time_to_tm_local(timestamp_t time) ^ date.c: In function ‘show_date’: date.c:211:8: error: implicit declaration of function ‘time_to_tm_local’ [-Werror=implicit-function-declaration] tm = time_to_tm_local(time); ^ date.c:211:6: error: assignment makes pointer from integer without a cast [-Werror=int-conversion] tm = time_to_tm_local(time); ^ cc1: all warnings being treated as errors -- snap -- I would expect this to be rebased *at least* to dddbad728c9 (timestamp_t: a new data type for timestamps, 2017-04-26). But even then, it fails in t0006 on Windows with this error: -- snip -- ++ eval 'diff -u "$@" ' +++ diff -u expect actual --- expect 2017-06-14 10:53:40.126136900 + +++ actual 2017-06-14 10:53:40.171146800 + @@ -1 +1 @@ -146600 +0200 -> 2016-06-15 14:13:20 + (UTC) +146600 +0200 -> 2016-06-15 14:13:20 UTC (UTC) + test_eval_ret_=1 + want_trace + test t = t + test t = t + set +x error: last command exited with $?=1 not ok 23 - show date (format-local:%Y-%m-%d %H:%M:%S %z (%Z):146600 +0200) # # echo "$time -> $expect" >expect && # ( # if test -n "$zone" # then # TZ=$zone # export $TZ # fi && # test-date show:"$format" "$time" >actual # ) && # test_cmp expect actual -- snap -- What gives? Ciao, Dscho
Re: What's cooking in git.git (Jun 2017, #04; Tue, 13)
> On 13 Jun 2017, at 23:40, Junio C Hamano wrote: > > > * ls/github (2017-06-13) 1 commit > - Configure Git contribution guidelines for github.com > > Help contributors that visit us at GitHub. > > Will merge to 'next'. I just pushed v3. If possible, please use it for 'next': http://public-inbox.org/git/20170614073536.45498-1-larsxschnei...@gmail.com/ > * ls/filter-process-delayed (2017-06-01) 5 commits > - convert: add "status=delayed" to filter process protocol > - convert: move multiple file filter error handling to separate function > - t0021: write "OUT" only on success > - t0021: make debug log file name configurable > - t0021: keep filter log files on comparison > > The filter-process interface learned to allow a process with long > latency give a "delayed" response. > > Needs review. I wonder if anyone has a few free cycles to review this: http://public-inbox.org/git/20170601082203.50397-1-larsxschnei...@gmail.com/ My GitLFS users are desperately waiting for this change :-) Thank you, Lars
Re: What's cooking in git.git (Jun 2017, #04; Tue, 13)
Jun 13, 2017 at 02:40:16PM -0700, Junio C Hamano wrote: > * sb/submodule-blanket-recursive (2017-06-01) 9 commits > (merged to 'next' on 2017-06-04 at 418bb03032) > + builtin/fetch.c: respect 'submodule.recurse' option > + builtin/push.c: respect 'submodule.recurse' option > + builtin/grep.c: respect 'submodule.recurse' option > + Introduce 'submodule.recurse' option for worktree manipulators > + submodule loading: separate code path for .gitmodules and config overlay > + reset/checkout/read-tree: unify config callback for submodule recursion > + submodule test invocation: only pass additional arguments > + submodule recursing: do not write a config variable twice > + Merge branch 'ab/grep-preparatory-cleanup' into > sb/submodule-blanket-recursive > > Many commands learned to pay attention to submodule.recurse > configuration. Yay! > It is not known if a simple "yes/no" is sufficient in the longer > term, and what should happen when --recurse-submodules option starts > taking "recurse into them how?" parameter, though. Any pointers for where this has been discussed, if anywhere (e.g. was it in the thread http://public-inbox.org/git/20170526191017.19155-1-sbel...@google.com)? I'm hoping that we can make the defaults work well enough that a simple "true/false" becomes sufficient. Perhaps this is something that the documentation at http://public-inbox.org/git/20170607185354.10050-1-sbel...@google.com can go into, since it is an opinionated piece of documentation that describes commonalities between submodule-related commands and how they are meant to fit into a user's daily life. [...] > * bw/config-h (2017-06-13) 4 commits > - config: don't implicitly use gitdir > - config: don't include config.h by default > - config: remove git_config_iter > - config: create config.h > > Code clean-up. Patches 1-3 are good to go IMHO. Patch 4 in pu is marked with my Reviewed-by. I think it's getting there but not there yet. Did some script pull the tag from my reply to the cover letter? (I'm asking so that if so I can cooperate better with such a script in the future and avoid false positive Reviewed-bys.) [...] > * jk/warn-add-gitlink (2017-06-13) 2 commits > - t: move "git add submodule" into test blocks > - add: warn when adding an embedded repository > > Using "git add d/i/r" when d/i/r is the top of the working tree of > a separate repository would create a gitlink in the index, which > would appear as a not-quite-initialized submodule to others. We > learned to give warnings when this happens. Note to self that we may want to put a note about this (and more generally about the git-series style of caller that creates a GITLINK entry that is not for a submodule) in the document being written at http://public-inbox.org/git/20170607185354.10050-1-sbel...@google.com or in some other document like gitrepository-layout.txt. [...] > * ls/github (2017-06-13) 1 commit > - Configure Git contribution guidelines for github.com > > Help contributors that visit us at GitHub. > > Will merge to 'next'. \o/ Thank you. [...] > -- > [Stalled] > > * mg/status-in-progress-info (2017-05-10) 2 commits > - status --short --inprogress: spell it as --in-progress > - status: show in-progress info for short status > > "git status" learns an option to report various operations > (e.g. "merging") that the user is in the middle of. > > cf. Thanks for the poke. This looks a quite nice change, but I agree with you about its current state. Regards, Jonathan
What's cooking in git.git (Jun 2017, #04; Tue, 13)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to "master"] * jc/diff-tree-stale-comment (2017-06-02) 1 commit (merged to 'next' on 2017-06-04 at bffae281d2) + diff-tree: update stale in-code comments Comment fix. * jc/noent-notdir (2017-05-30) 2 commits (merged to 'next' on 2017-06-04 at 7cb4efbc3c) + treewide: use is_missing_file_error() where ENOENT and ENOTDIR are checked + compat-util: is_missing_file_error() Our code often opens a path to an optional file, to work on its contents when we can successfully open it. We can ignore a failure to open if such an optional file does not exist, but we do want to report a failure in opening for other reasons (e.g. we got an I/O error, or the file is there, but we lack the permission to open). The exact errors we need to ignore are ENOENT (obviously) and ENOTDIR (less obvious). Instead of repeating comparison of errno with these two constants, introduce a helper function to do so. * jk/pack-idx-corruption-safety (2017-06-07) 1 commit (merged to 'next' on 2017-06-07 at 31f94e174d) + t5313: make extended-table test more deterministic A flaky test has been corrected. * nd/fopen-errors (2017-06-02) 13 commits (merged to 'next' on 2017-06-04 at 7a755e73bb) + mingw_fopen: report ENOENT for invalid file names + mingw: verify that paths are not mistaken for remote nicknames + log: fix memory leak in open_next_file() + rerere.c: move error_errno() closer to the source system call + print errno when reporting a system call error + wrapper.c: make warn_on_inaccessible() static + wrapper.c: add and use fopen_or_warn() + wrapper.c: add and use warn_on_fopen_errors() + config.mak.uname: set FREAD_READS_DIRECTORIES for Darwin, too + config.mak.uname: set FREAD_READS_DIRECTORIES for Linux and FreeBSD + clone: use xfopen() instead of fopen() + use xfopen() in more places + git_fopen: fix a sparse 'not declared' warning We often try to open a file for reading whose existence is optional, and silently ignore errors from open/fopen; report such errors if they are not due to missing files. * rf/completion (2017-06-02) 6 commits (merged to 'next' on 2017-06-04 at dde1e34703) + completion: add git config credentialCache.ignoreSIGHUP + completion: add git config credential completions + completion: add git config advice completions + completion: add git config am.threeWay completion + completion: add git config core completions + completion: add git config gc completions Completion updates. * sb/submodule-blanket-recursive (2017-06-01) 9 commits (merged to 'next' on 2017-06-04 at 418bb03032) + builtin/fetch.c: respect 'submodule.recurse' option + builtin/push.c: respect 'submodule.recurse' option + builtin/grep.c: respect 'submodule.recurse' option + Introduce 'submodule.recurse' option for worktree manipulators + submodule loading: separate code path for .gitmodules and config overlay + reset/checkout/read-tree: unify config callback for submodule recursion + submodule test invocation: only pass additional arguments + submodule recursing: do not write a config variable twice + Merge branch 'ab/grep-preparatory-cleanup' into sb/submodule-blanket-recursive Many commands learned to pay attention to submodule.recurse configuration. It is not known if a simple "yes/no" is sufficient in the longer term, and what should happen when --recurse-submodules option starts taking "recurse into them how?" parameter, though. -- [New Topics] * js/alias-early-config (2017-06-13) 6 commits - Use the early config machinery to expand aliases - t7006: demonstrate a problem with aliases in subdirectories - t1308: relax the test verifying that empty alias values are disallowed - help: use early config when autocorrecting aliases - config: report correct line number upon error - discover_git_directory(): avoid setting invalid git_dir The code to pick up and execute command alias definition from the configuration used to switch to the top of the working tree and then come back when the expanded alias was executed, which was unnecessarilyl complex. Attempt to simplify the logic by using the early-config mechanism that does not chdir around. Waiting for discussion to settle. * pc/dir-count-slashes (2017-06-12) 1 commit - dir: create function count_slashes() Three instances of the same helper function have been consolidated to one. Will merge to 'next'. * sb/t4005-modernize (2017-06-10) 1 commit - t4005: modernize styl
