* Bug: rebase -i creates committer time inversions on 'reword' @ 2018-04-13 16:52 Johannes Sixt 2018-04-14 11:15 ` Phillip Wood ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Johannes Sixt @ 2018-04-13 16:52 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Git Mailing List I just noticed that all commits in a 70-commit branch have the same committer timestamp. This is very unusual on Windows, where rebase -i of such a long branch takes more than one second (but not more than 3 or so thanks to the builtin nature of the command!). And, in fact, if you mark some commits with 'reword' to delay the quick processing of the patches, then the reworded commits have later time stamps, but subsequent not reworded commits receive the earlier time stamp. This is clearly not intended. Perhaps something like this below is needed. diff --git a/ident.c b/ident.c index 327abe557f..2c6bff7b9d 100644 --- a/ident.c +++ b/ident.c @@ -178,8 +178,8 @@ const char *ident_default_email(void) static const char *ident_default_date(void) { - if (!git_default_date.len) - datestamp(&git_default_date); + strbuf_reset(&git_default_date); + datestamp(&git_default_date); return git_default_date.buf; } ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: Bug: rebase -i creates committer time inversions on 'reword' 2018-04-13 16:52 Bug: rebase -i creates committer time inversions on 'reword' Johannes Sixt @ 2018-04-14 11:15 ` Phillip Wood 2018-04-14 13:11 ` Johannes Schindelin 2018-04-15 21:35 ` Junio C Hamano 2018-04-18 10:22 ` [RFC PATCH] ident: don't cache default date Phillip Wood 2 siblings, 1 reply; 17+ messages in thread From: Phillip Wood @ 2018-04-14 11:15 UTC (permalink / raw) To: Johannes Sixt, Johannes Schindelin; +Cc: Git Mailing List On 13/04/18 17:52, Johannes Sixt wrote: > > I just noticed that all commits in a 70-commit branch have the same > committer timestamp. This is very unusual on Windows, where rebase -i of > such a long branch takes more than one second (but not more than 3 or > so thanks to the builtin nature of the command!). > > And, in fact, if you mark some commits with 'reword' to delay the quick > processing of the patches, then the reworded commits have later time > stamps, but subsequent not reworded commits receive the earlier time > stamp. This is clearly not intended. Oh dear, I think this is probably due to my series making rebase commit in-process when the commit message isn't being edited. I didn't realize that git cached the commit date rather than using the current time when calling commit_tree_extended(). I'll take a look at it next week. I think 'git am' probably gives all patches the same commit time as well if the commit date is cached though it wont suffer from the time-travel problem. Best Wishes Phillip > Perhaps something like this below is needed. > > diff --git a/ident.c b/ident.c > index 327abe557f..2c6bff7b9d 100644 > --- a/ident.c > +++ b/ident.c > @@ -178,8 +178,8 @@ const char *ident_default_email(void) > > static const char *ident_default_date(void) > { > - if (!git_default_date.len) > - datestamp(&git_default_date); > + strbuf_reset(&git_default_date); > + datestamp(&git_default_date); > return git_default_date.buf; > } > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Bug: rebase -i creates committer time inversions on 'reword' 2018-04-14 11:15 ` Phillip Wood @ 2018-04-14 13:11 ` Johannes Schindelin 2018-04-16 9:48 ` Phillip Wood 0 siblings, 1 reply; 17+ messages in thread From: Johannes Schindelin @ 2018-04-14 13:11 UTC (permalink / raw) To: phillip.wood; +Cc: Johannes Sixt, Git Mailing List Hi, On Sat, 14 Apr 2018, Phillip Wood wrote: > On 13/04/18 17:52, Johannes Sixt wrote: > > > > I just noticed that all commits in a 70-commit branch have the same > > committer timestamp. This is very unusual on Windows, where rebase -i of > > such a long branch takes more than one second (but not more than 3 or > > so thanks to the builtin nature of the command!). > > > > And, in fact, if you mark some commits with 'reword' to delay the quick > > processing of the patches, then the reworded commits have later time > > stamps, but subsequent not reworded commits receive the earlier time > > stamp. This is clearly not intended. > > Oh dear, I think this is probably due to my series making rebase commit > in-process when the commit message isn't being edited. I didn't realize > that git cached the commit date rather than using the current time when > calling commit_tree_extended(). I'll take a look at it next week. Thanks. However, a quick lock at `git log @{u}.. --format=%ct` in my `sequencer-shears` branch thicket (which I rebase frequently on top of upstream's `master` using the last known-good `rebase-merges` sub-branch) shows that the commits have different-enough commit timestamps. (It is satisfying to see that multiple commits were made during the same second, of course.) So while I cannot find anything in the code that disagrees with Hannes' assessment, it looks on the surface as if I did not encounter the bug here. Curious. FWIW I agree with Hannes' patch. > I think 'git am' probably gives all patches the same commit time as well > if the commit date is cached though it wont suffer from the time-travel > problem. I thought that `git am` was the subject of such a complaint recently, but I thought that had been resolved? Apparently I misremember... Ciao, Dscho ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Bug: rebase -i creates committer time inversions on 'reword' 2018-04-14 13:11 ` Johannes Schindelin @ 2018-04-16 9:48 ` Phillip Wood 2018-04-19 9:17 ` Phillip Wood 0 siblings, 1 reply; 17+ messages in thread From: Phillip Wood @ 2018-04-16 9:48 UTC (permalink / raw) To: Johannes Schindelin, phillip.wood; +Cc: Johannes Sixt, Git Mailing List On 14/04/18 14:11, Johannes Schindelin wrote: > Hi, > > On Sat, 14 Apr 2018, Phillip Wood wrote: > >> On 13/04/18 17:52, Johannes Sixt wrote: >>> >>> I just noticed that all commits in a 70-commit branch have the same >>> committer timestamp. This is very unusual on Windows, where rebase -i of >>> such a long branch takes more than one second (but not more than 3 or >>> so thanks to the builtin nature of the command!). >>> >>> And, in fact, if you mark some commits with 'reword' to delay the quick >>> processing of the patches, then the reworded commits have later time >>> stamps, but subsequent not reworded commits receive the earlier time >>> stamp. This is clearly not intended. >> >> Oh dear, I think this is probably due to my series making rebase commit >> in-process when the commit message isn't being edited. I didn't realize >> that git cached the commit date rather than using the current time when >> calling commit_tree_extended(). I'll take a look at it next week. > > Thanks. > > However, a quick lock at `git log @{u}.. --format=%ct` in my > `sequencer-shears` branch thicket (which I rebase frequently on top of > upstream's `master` using the last known-good `rebase-merges` sub-branch) > shows that the commits have different-enough commit timestamps. (It is > satisfying to see that multiple commits were made during the same second, > of course.) > > So while I cannot find anything in the code that disagrees with Hannes' > assessment, it looks on the surface as if I did not encounter the bug > here. > > Curious. That's strange (I'd have expected the picks after recreated merges to have the earlier timestamps than the merge), if I do 'git rebase -i --force-rebase --exec="sleep 2" @~5' then all the new commits have the same timestamp. > FWIW I agree with Hannes' patch. > >> I think 'git am' probably gives all patches the same commit time as well >> if the commit date is cached though it wont suffer from the time-travel >> problem. > > I thought that `git am` was the subject of such a complaint recently, but > I thought that had been resolved? Apparently I misremember... I had a quick look and couldn't see anything about that, it looks to me like it just calls commit_tree() and only does anything to change the default commit date if '--committer-date-is-author-date' was given. Best Wishes Phillip > Ciao, > Dscho > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Bug: rebase -i creates committer time inversions on 'reword' 2018-04-16 9:48 ` Phillip Wood @ 2018-04-19 9:17 ` Phillip Wood 0 siblings, 0 replies; 17+ messages in thread From: Phillip Wood @ 2018-04-19 9:17 UTC (permalink / raw) To: Johannes Schindelin, phillip.wood; +Cc: Johannes Sixt, Git Mailing List On 16/04/18 10:48, Phillip Wood wrote: > On 14/04/18 14:11, Johannes Schindelin wrote: >> Hi, >> >> On Sat, 14 Apr 2018, Phillip Wood wrote: >> >> FWIW I agree with Hannes' patch. >> >>> I think 'git am' probably gives all patches the same commit time as well >>> if the commit date is cached though it wont suffer from the time-travel >>> problem. >> >> I thought that `git am` was the subject of such a complaint recently, but >> I thought that had been resolved? Apparently I misremember... > > I had a quick look and couldn't see anything about that, it looks to me > like it just calls commit_tree() and only does anything to change the > default commit date if '--committer-date-is-author-date' was given. Ah you were right, I just didn't look far enough back in the history .(it's a shame it calls reset_ident_date() in a different function to the one that creates the commit otherwise I would probably have noticed it when I wrote the original patches) Best Wishes Phillip > > Best Wishes > > Phillip >> Ciao, >> Dscho >> > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Bug: rebase -i creates committer time inversions on 'reword' 2018-04-13 16:52 Bug: rebase -i creates committer time inversions on 'reword' Johannes Sixt 2018-04-14 11:15 ` Phillip Wood @ 2018-04-15 21:35 ` Junio C Hamano 2018-04-16 5:56 ` Johannes Sixt 2018-04-18 10:22 ` [RFC PATCH] ident: don't cache default date Phillip Wood 2 siblings, 1 reply; 17+ messages in thread From: Junio C Hamano @ 2018-04-15 21:35 UTC (permalink / raw) To: Johannes Sixt; +Cc: Johannes Schindelin, Git Mailing List Johannes Sixt <j6t@kdbg.org> writes: > I just noticed that all commits in a 70-commit branch have the same > committer timestamp. This is very unusual on Windows, where rebase -i of > such a long branch takes more than one second (but not more than 3 or > so thanks to the builtin nature of the command!). > > And, in fact, if you mark some commits with 'reword' to delay the quick > processing of the patches, then the reworded commits have later time > stamps, but subsequent not reworded commits receive the earlier time > stamp. This is clearly not intended. Hmm, I may be missing something without enough caffeine but I am puzzled how that would be possible. With a "few picks, an edit, and a yet more picks" sequence, the first picks may share the same timestamp due to the git_default_date caching (which I think is a deliberate design choice we made), an edit that stops will let the concluding "commit" (either by the end user or invoked internally via "rebase --continue"), but because that process restarts afresh, the commits made by "yet more picks" cannot share the timestamp that was cached for the earliest ones from the same series, no? Ah, do you mean we have an internal sequence like this, when "rebase --continue" wants to conclude an edit/reword? - we figure out the committer ident, which grabs a timestamp and cache it; - we spawn "commit" to conclude the stopped step, letting it record its beginning time (which is a bit older than the above) or its ending time (which is much older due to human typing speed); - subsequent "picks" are made in the same process, and share the timestamp we grabbed in the first step, which is older than the second one. I guess we'd want a mechanism to tell ident.c layer "discard the cached one, as we are no longer in the same automated sequence", and use that whenever we spawn an editor (or otherwise go interactive). > > Perhaps something like this below is needed. > > diff --git a/ident.c b/ident.c > index 327abe557f..2c6bff7b9d 100644 > --- a/ident.c > +++ b/ident.c > @@ -178,8 +178,8 @@ const char *ident_default_email(void) > > static const char *ident_default_date(void) > { > - if (!git_default_date.len) > - datestamp(&git_default_date); > + strbuf_reset(&git_default_date); > + datestamp(&git_default_date); > return git_default_date.buf; > } > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Bug: rebase -i creates committer time inversions on 'reword' 2018-04-15 21:35 ` Junio C Hamano @ 2018-04-16 5:56 ` Johannes Sixt 2018-04-17 1:37 ` Junio C Hamano 2018-04-18 10:19 ` Phillip Wood 0 siblings, 2 replies; 17+ messages in thread From: Johannes Sixt @ 2018-04-16 5:56 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin, Git Mailing List Am 15.04.2018 um 23:35 schrieb Junio C Hamano: > Ah, do you mean we have an internal sequence like this, when "rebase > --continue" wants to conclude an edit/reword? Yes, it's only 'reword' that is affected, because then subsequent picks are processed by the original process. > - we figure out the committer ident, which grabs a timestamp and > cache it; > > - we spawn "commit" to conclude the stopped step, letting it record > its beginning time (which is a bit older than the above) or its > ending time (which is much older due to human typing speed); Younger in both cases, of course. According to my tests, we seem to pick the beginning time, because the first 'reword'ed commit typically has the same timestamp as the preceding picks. Later 'reword'ed commits have noticably younger timestamps. > - subsequent "picks" are made in the same process, and share the > timestamp we grabbed in the first step, which is older than the > second one. > > I guess we'd want a mechanism to tell ident.c layer "discard the > cached one, as we are no longer in the same automated sequence", and > use that whenever we spawn an editor (or otherwise go interactive). Frankly, I think that this caching is overengineered (or prematurly optimized). If the design requires that different callers of datestamp() must see the same time, then the design is broken. In a fixed design, there would be a single call of datestamp() in advance, and then the timestamp, which then obviously is a very important piece of data, would be passed along as required. -- Hannes ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Bug: rebase -i creates committer time inversions on 'reword' 2018-04-16 5:56 ` Johannes Sixt @ 2018-04-17 1:37 ` Junio C Hamano 2018-04-18 10:19 ` Phillip Wood 1 sibling, 0 replies; 17+ messages in thread From: Junio C Hamano @ 2018-04-17 1:37 UTC (permalink / raw) To: Johannes Sixt; +Cc: Johannes Schindelin, Git Mailing List Johannes Sixt <j6t@kdbg.org> writes: > Am 15.04.2018 um 23:35 schrieb Junio C Hamano: >> Ah, do you mean we have an internal sequence like this, when "rebase >> --continue" wants to conclude an edit/reword? > > Yes, it's only 'reword' that is affected, because then subsequent > picks are processed by the original process. Ah, OK, that is a good explanation. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Bug: rebase -i creates committer time inversions on 'reword' 2018-04-16 5:56 ` Johannes Sixt 2018-04-17 1:37 ` Junio C Hamano @ 2018-04-18 10:19 ` Phillip Wood 1 sibling, 0 replies; 17+ messages in thread From: Phillip Wood @ 2018-04-18 10:19 UTC (permalink / raw) To: Johannes Sixt, Junio C Hamano; +Cc: Johannes Schindelin, Git Mailing List On 16/04/18 06:56, Johannes Sixt wrote: > > Am 15.04.2018 um 23:35 schrieb Junio C Hamano: >> Ah, do you mean we have an internal sequence like this, when "rebase >> --continue" wants to conclude an edit/reword? > > Yes, it's only 'reword' that is affected, because then subsequent picks > are processed by the original process. > >> - we figure out the committer ident, which grabs a timestamp and >> cache it; >> >> - we spawn "commit" to conclude the stopped step, letting it record >> its beginning time (which is a bit older than the above) or its >> ending time (which is much older due to human typing speed); > > Younger in both cases, of course. According to my tests, we seem to pick > the beginning time, because the first 'reword'ed commit typically has > the same timestamp as the preceding picks. Later 'reword'ed commits have > noticably younger timestamps. > >> - subsequent "picks" are made in the same process, and share the >> timestamp we grabbed in the first step, which is older than the >> second one. >> >> I guess we'd want a mechanism to tell ident.c layer "discard the >> cached one, as we are no longer in the same automated sequence", and >> use that whenever we spawn an editor (or otherwise go interactive). > > Frankly, I think that this caching is overengineered (or prematurly > optimized). If the design requires that different callers of datestamp() > must see the same time, then the design is broken. In a fixed design, > there would be a single call of datestamp() in advance, and then the > timestamp, which then obviously is a very important piece of data, would > be passed along as required. I'm inclined to agree, though it creates complications if we're going to keep giving commits the same author and committer dates when neither is explicitly specified. Best Wishes Phillip > > -- Hannes ^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH] ident: don't cache default date 2018-04-13 16:52 Bug: rebase -i creates committer time inversions on 'reword' Johannes Sixt 2018-04-14 11:15 ` Phillip Wood 2018-04-15 21:35 ` Junio C Hamano @ 2018-04-18 10:22 ` Phillip Wood 2018-04-18 11:27 ` Ævar Arnfjörð Bjarmason 2 siblings, 1 reply; 17+ messages in thread From: Phillip Wood @ 2018-04-18 10:22 UTC (permalink / raw) To: Johannes Sixt, Johannes Schindelin, Junio C Hamano Cc: Git Mailing List, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> Now that the sequencer commits without forking when the commit message isn't edited all the commits that are picked have the same committer date. If a commit is reworded it's committer date will be a later time as it is created by running an separate instance of 'git commit'. If the reworded commit is follow by further picks, those later commits will have an earlier committer date than the reworded one. This is caused by git caching the default date used when GIT_COMMITTER_DATE is not set. Fix this by not caching the date. Users expect commits to have the same author and committer dates when the don't explicitly set them. As the date is now updated each time git_author_info() or git_committer_info() is run it is possible to end up with different author and committer dates. Fix this for 'commit-tree', 'notes' and 'merge' by using a single date in commit_tree_extended() and passing it explicitly to the new functions git_author_info_with_date() and git_committer_info_with_date() when neither the author date nor the committer date are explicitly set. 'commit' always passes the author date to commit_tree_extended() and relied on the date caching to have the same committer and author dates when neither was specified. Fix this by setting GIT_COMMITTER_DATE to be the same as the author date passed to commit_tree_extended(). Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Reported-by: Johannes Sixt <j6t@kdbg.org> --- I'm slightly nervous that setting GIT_COMMITTER_DATE in builtin/commit.c will break someone's hook script. Maybe it would be better to add a committer parameter to commit_tree() and commit_tree_extended(). This needs some tests. I think we could test that the sequencer gives each commit a new date with 'git rebase -i --exec="sleep 2" --force-rebase @~2' and test the committer dates of the rebased commits. I'm not sure if test -gt can cope with numbers that big though, maybe we'll have to be content with test !=. For 'git commit' I think doing GIT_EDITOR='sleep 2; echo message >"$1"' and checking the committer date and author date will work as the author date is set before the user edits the commit message. I'm not sure how to test callers of commit_tree() though (that's commit-tree, notes and merge) as I've not been able to come up with anything that will guarantee the author and committer dates are different if the code in commit_tree_extended() that ensures they are the same gets broken. builtin/commit.c | 8 ++++++++ cache.h | 2 ++ commit.c | 22 +++++++++++++++++++--- ident.c | 24 ++++++++++++++++++------ 4 files changed, 47 insertions(+), 9 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 37fcb55ab0..4ad8317f88 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -584,6 +584,14 @@ static void determine_author_info(struct strbuf *author_ident) export_one("GIT_AUTHOR_NAME", author.name_begin, author.name_end, 0); export_one("GIT_AUTHOR_EMAIL", author.mail_begin, author.mail_end, 0); export_one("GIT_AUTHOR_DATE", author.date_begin, author.tz_end, '@'); + /* + * Ensure the author and committer dates are identical if nither is + * explicitly set + */ + if ((!date || !*date) && (!getenv("GIT_COMMITTER_DATE") + || !*getenv("GIT_COMMITTER_DATE"))) + export_one("GIT_COMMITTER_DATE", author.date_begin, + author.tz_end, '@'); free(name); free(email); free(date); diff --git a/cache.h b/cache.h index a61b2d3f0d..9cde499507 100644 --- a/cache.h +++ b/cache.h @@ -1484,7 +1484,9 @@ int date_overflows(timestamp_t date); #define IDENT_NO_DATE 2 #define IDENT_NO_NAME 4 extern const char *git_author_info(int); +extern const char *git_author_info_with_date(int, const char*); extern const char *git_committer_info(int); +extern const char *git_committer_info_with_date(int, const char*); extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int); extern const char *fmt_name(const char *name, const char *email); extern const char *ident_default_name(void); diff --git a/commit.c b/commit.c index 00c99c7272..457cc8b71b 100644 --- a/commit.c +++ b/commit.c @@ -1513,6 +1513,7 @@ int commit_tree_extended(const char *msg, size_t msg_len, const char *author, const char *sign_commit, struct commit_extra_header *extra) { + struct strbuf date_buf = STRBUF_INIT; int result; int encoding_is_utf8; struct strbuf buffer; @@ -1540,10 +1541,25 @@ int commit_tree_extended(const char *msg, size_t msg_len, } /* Person/date information */ - if (!author) - author = git_author_info(IDENT_STRICT); + if (!author) { + char *author_date = xstrdup_or_null(getenv("GIT_AUTHOR_DATE")); + char *committer_date = + xstrdup_or_null(getenv("GIT_COMMITTER_DATE")); + /* + * If neither the author nor committer dates are explicitly set + * ensure they are identical + */ + if (!author_date && !committer_date) { + datestamp(&date_buf); + } + author = git_author_info_with_date(IDENT_STRICT, date_buf.buf); + free(author_date); + free(committer_date); + } strbuf_addf(&buffer, "author %s\n", author); - strbuf_addf(&buffer, "committer %s\n", git_committer_info(IDENT_STRICT)); + strbuf_addf(&buffer, "committer %s\n", + git_committer_info_with_date(IDENT_STRICT, date_buf.buf)); + strbuf_release(&date_buf); if (!encoding_is_utf8) strbuf_addf(&buffer, "encoding %s\n", git_commit_encoding); diff --git a/ident.c b/ident.c index 327abe557f..f9eb933c2c 100644 --- a/ident.c +++ b/ident.c @@ -178,8 +178,8 @@ const char *ident_default_email(void) static const char *ident_default_date(void) { - if (!git_default_date.len) - datestamp(&git_default_date); + strbuf_reset(&git_default_date); + datestamp(&git_default_date); return git_default_date.buf; } @@ -427,30 +427,42 @@ const char *fmt_name(const char *name, const char *email) return fmt_ident(name, email, NULL, IDENT_STRICT | IDENT_NO_DATE); } -const char *git_author_info(int flag) +const char *git_author_info_with_date(int flag, const char *date_str) { if (getenv("GIT_AUTHOR_NAME")) author_ident_explicitly_given |= IDENT_NAME_GIVEN; if (getenv("GIT_AUTHOR_EMAIL")) author_ident_explicitly_given |= IDENT_MAIL_GIVEN; return fmt_ident(getenv("GIT_AUTHOR_NAME"), getenv("GIT_AUTHOR_EMAIL"), - getenv("GIT_AUTHOR_DATE"), + date_str && *date_str ? date_str : + getenv("GIT_AUTHOR_DATE"), flag); } -const char *git_committer_info(int flag) +const char *git_author_info(int flag) +{ + return git_author_info_with_date(flag, NULL); +} + +const char *git_committer_info_with_date(int flag, const char *date_str) { if (getenv("GIT_COMMITTER_NAME")) committer_ident_explicitly_given |= IDENT_NAME_GIVEN; if (getenv("GIT_COMMITTER_EMAIL")) committer_ident_explicitly_given |= IDENT_MAIL_GIVEN; return fmt_ident(getenv("GIT_COMMITTER_NAME"), getenv("GIT_COMMITTER_EMAIL"), - getenv("GIT_COMMITTER_DATE"), + date_str && *date_str ? date_str : + getenv("GIT_COMMITTER_DATE"), flag); } +const char *git_committer_info(int flag) +{ + return git_committer_info_with_date(flag, NULL); +} + static int ident_is_sufficient(int user_ident_explicitly_given) { #ifndef WINDOWS -- 2.17.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [RFC PATCH] ident: don't cache default date 2018-04-18 10:22 ` [RFC PATCH] ident: don't cache default date Phillip Wood @ 2018-04-18 11:27 ` Ævar Arnfjörð Bjarmason 2018-04-18 17:47 ` Phillip Wood 0 siblings, 1 reply; 17+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2018-04-18 11:27 UTC (permalink / raw) To: Phillip Wood Cc: Johannes Sixt, Johannes Schindelin, Junio C Hamano, Git Mailing List On Wed, Apr 18 2018, Phillip Wood wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > Now that the sequencer commits without forking when the commit message > isn't edited all the commits that are picked have the same committer > date. If a commit is reworded it's committer date will be a later time s/it's/its/ > as it is created by running an separate instance of 'git commit'. If > the reworded commit is follow by further picks, those later commits > will have an earlier committer date than the reworded one. This is > caused by git caching the default date used when GIT_COMMITTER_DATE is > not set. Fix this by not caching the date. > > Users expect commits to have the same author and committer dates when > the don't explicitly set them. As the date is now updated each time > git_author_info() or git_committer_info() is run it is possible to end > up with different author and committer dates. Fix this for > 'commit-tree', 'notes' and 'merge' by using a single date in > commit_tree_extended() and passing it explicitly to the new functions > git_author_info_with_date() and git_committer_info_with_date() when > neither the author date nor the committer date are explicitly > set. 'commit' always passes the author date to commit_tree_extended() > and relied on the date caching to have the same committer and author > dates when neither was specified. Fix this by setting > GIT_COMMITTER_DATE to be the same as the author date passed to > commit_tree_extended(). > > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > Reported-by: Johannes Sixt <j6t@kdbg.org> > --- > > I'm slightly nervous that setting GIT_COMMITTER_DATE in > builtin/commit.c will break someone's hook script. Maybe it would be > better to add a committer parameter to commit_tree() and > commit_tree_extended(). > > This needs some tests. I think we could test that the sequencer gives > each commit a new date with 'git rebase -i --exec="sleep 2" > --force-rebase @~2' and test the committer dates of the rebased > commits. I'm not sure if test -gt can cope with numbers that big > though, maybe we'll have to be content with test !=. For 'git commit' > I think doing GIT_EDITOR='sleep 2; echo message >"$1"' and checking > the committer date and author date will work as the author date is set > before the user edits the commit message. I'm not sure how to test > callers of commit_tree() though (that's commit-tree, notes and merge) > as I've not been able to come up with anything that will guarantee the > author and committer dates are different if the code in > commit_tree_extended() that ensures they are the same gets broken. The behavior you're describing where we end up with later commits in the rebase with earlier CommitDate's definitely sounds like a minor regression, and yeah we should have tests for this. My mental model of this is that we shouldn't be trying at all to adjust the CommitDate in a sequence to be the same, and just make it be whatever time() returns, except in the case where a date is passed explicitly. My cursory reading of this RFC patch is that it's definitely an improvement because we don't end up with things out of order, but is there any reason for why we should be trying to aim to keep this "consistent" within a single "git rebase" command by default, as opposed to always just writing the current CommitDate at the time we make the commit, that sounds like the most intuitive thing to me, and I can't see any downsides with that. It leaks info about how long it took someone to do the rebase, but so what? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH] ident: don't cache default date 2018-04-18 11:27 ` Ævar Arnfjörð Bjarmason @ 2018-04-18 17:47 ` Phillip Wood 2018-04-18 18:15 ` Johannes Sixt 0 siblings, 1 reply; 17+ messages in thread From: Phillip Wood @ 2018-04-18 17:47 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason, Phillip Wood Cc: Johannes Sixt, Johannes Schindelin, Junio C Hamano, Git Mailing List Hi Ævar, thanks for your comments On 18/04/18 12:27, Ævar Arnfjörð Bjarmason wrote: > > On Wed, Apr 18 2018, Phillip Wood wrote: > >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> Now that the sequencer commits without forking when the commit message >> isn't edited all the commits that are picked have the same committer >> date. If a commit is reworded it's committer date will be a later time > > s/it's/its/ Thanks I'll change it >> as it is created by running an separate instance of 'git commit'. If >> the reworded commit is follow by further picks, those later commits >> will have an earlier committer date than the reworded one. This is >> caused by git caching the default date used when GIT_COMMITTER_DATE is >> not set. Fix this by not caching the date. >> >> Users expect commits to have the same author and committer dates when >> the don't explicitly set them. As the date is now updated each time >> git_author_info() or git_committer_info() is run it is possible to end >> up with different author and committer dates. Fix this for >> 'commit-tree', 'notes' and 'merge' by using a single date in >> commit_tree_extended() and passing it explicitly to the new functions >> git_author_info_with_date() and git_committer_info_with_date() when >> neither the author date nor the committer date are explicitly >> set. 'commit' always passes the author date to commit_tree_extended() >> and relied on the date caching to have the same committer and author >> dates when neither was specified. Fix this by setting >> GIT_COMMITTER_DATE to be the same as the author date passed to >> commit_tree_extended(). >> >> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> >> Reported-by: Johannes Sixt <j6t@kdbg.org> >> --- >> >> I'm slightly nervous that setting GIT_COMMITTER_DATE in >> builtin/commit.c will break someone's hook script. Maybe it would be >> better to add a committer parameter to commit_tree() and >> commit_tree_extended(). >> >> This needs some tests. I think we could test that the sequencer gives >> each commit a new date with 'git rebase -i --exec="sleep 2" >> --force-rebase @~2' and test the committer dates of the rebased >> commits. I'm not sure if test -gt can cope with numbers that big >> though, maybe we'll have to be content with test !=. For 'git commit' >> I think doing GIT_EDITOR='sleep 2; echo message >"$1"' and checking >> the committer date and author date will work as the author date is set >> before the user edits the commit message. I'm not sure how to test >> callers of commit_tree() though (that's commit-tree, notes and merge) >> as I've not been able to come up with anything that will guarantee the >> author and committer dates are different if the code in >> commit_tree_extended() that ensures they are the same gets broken. > > The behavior you're describing where we end up with later commits in the > rebase with earlier CommitDate's definitely sounds like a minor > regression, and yeah we should have tests for this. > > My mental model of this is that we shouldn't be trying at all to adjust > the CommitDate in a sequence to be the same, and just make it be > whatever time() returns, except in the case where a date is passed > explicitly. > > My cursory reading of this RFC patch is that it's definitely an > improvement because we don't end up with things out of order, but is > there any reason for why we should be trying to aim to keep this > "consistent" within a single "git rebase" command by default, as opposed > to always just writing the current CommitDate at the time we make the > commit, that sounds like the most intuitive thing to me, and I can't see > any downsides with that. It's not trying to keep the date "consistent" within a single rebase command, each commit created by the sequencer gets a date stamp with the current time when the commit is created. What it is doing is keeping the author and committer dates the same for commit/commit-tree/merge/notes when the user does not specify either of them. While I don't think it particularly matters that they match (so long as the committer date is later than the author date), it is what git does at the moment and someone maybe relying on the current behavior. Best Wishes Phillip > > It leaks info about how long it took someone to do the rebase, but so > what? > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH] ident: don't cache default date 2018-04-18 17:47 ` Phillip Wood @ 2018-04-18 18:15 ` Johannes Sixt 2018-04-18 21:15 ` Junio C Hamano 2018-04-19 9:15 ` Phillip Wood 0 siblings, 2 replies; 17+ messages in thread From: Johannes Sixt @ 2018-04-18 18:15 UTC (permalink / raw) To: phillip.wood Cc: Ævar Arnfjörð Bjarmason, Johannes Schindelin, Junio C Hamano, Git Mailing List Am 18.04.2018 um 19:47 schrieb Phillip Wood: > On 18/04/18 12:27, Ævar Arnfjörð Bjarmason wrote: >> On Wed, Apr 18 2018, Phillip Wood wrote: >>> From: Phillip Wood <phillip.wood@dunelm.org.uk> >>> as it is created by running an separate instance of 'git commit'. If >>> the reworded commit is follow by further picks, those later commits >>> will have an earlier committer date than the reworded one. This is >>> caused by git caching the default date used when GIT_COMMITTER_DATE is >>> not set. Fix this by not caching the date. >>> >>> Users expect commits to have the same author and committer dates when >>> the don't explicitly set them. As the date is now updated each time >>> git_author_info() or git_committer_info() is run it is possible to end >>> up with different author and committer dates. Fix this for >>> 'commit-tree', 'notes' and 'merge' by using a single date in >>> commit_tree_extended() and passing it explicitly to the new functions >>> git_author_info_with_date() and git_committer_info_with_date() when >>> neither the author date nor the committer date are explicitly >>> set. 'commit' always passes the author date to commit_tree_extended() >>> and relied on the date caching to have the same committer and author >>> dates when neither was specified. Fix this by setting >>> GIT_COMMITTER_DATE to be the same as the author date passed to >>> commit_tree_extended(). >>> >>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> >>> Reported-by: Johannes Sixt <j6t@kdbg.org> >>> --- >>> >>> I'm slightly nervous that setting GIT_COMMITTER_DATE in >>> builtin/commit.c will break someone's hook script. Maybe it would be >>> better to add a committer parameter to commit_tree() and >>> commit_tree_extended(). While I like the basic theme of your patch, I think we should fix this case in a much simpler way, namely, use the infrastructure that was introduced for git-am. I've shamelessly lifted the commit message from your patch. ---- 8< ---- Subject: [PATCH] sequencer: reset the committer date before commits Now that the sequencer commits without forking when the commit message isn't edited all the commits that are picked have the same committer date. If a commit is reworded it's committer date will be a later time as it is created by running an separate instance of 'git commit'. If the reworded commit is follow by further picks, those later commits will have an earlier committer date than the reworded one. This is caused by git caching the default date used when GIT_COMMITTER_DATE is not set. Reset the cached date before a commit is generated in-process. Signed-off-by: Johannes Sixt <j6t@kdbg.org> --- sequencer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sequencer.c b/sequencer.c index f9d1001dee..f0bac903a0 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1148,6 +1148,8 @@ static int try_to_commit(struct strbuf *msg, const char *author, goto out; } + reset_ident_date(); + if (commit_tree_extended(msg->buf, msg->len, &tree, parents, oid, author, opts->gpg_sign, extra)) { res = error(_("failed to write commit object")); -- 2.17.0.69.g0c1d01d9b6 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [RFC PATCH] ident: don't cache default date 2018-04-18 18:15 ` Johannes Sixt @ 2018-04-18 21:15 ` Junio C Hamano 2018-04-19 9:15 ` Phillip Wood 1 sibling, 0 replies; 17+ messages in thread From: Junio C Hamano @ 2018-04-18 21:15 UTC (permalink / raw) To: Johannes Sixt Cc: phillip.wood, Ævar Arnfjörð Bjarmason, Johannes Schindelin, Git Mailing List Johannes Sixt <j6t@kdbg.org> writes: > While I like the basic theme of your patch, I think we should fix this > case in a much simpler way, namely, use the infrastructure that was > introduced for git-am. Yup. reset_ident_date() was introduced by 4d9c7e6f ("am: reset cached ident date for each patch", 2016-08-01) and the commit explains very well why it is a good idea to have both the caching and also the strategic resetting it introduces. Thanks, all. > I've shamelessly lifted the commit message from your patch. > > ---- 8< ---- > Subject: [PATCH] sequencer: reset the committer date before commits > > Now that the sequencer commits without forking when the commit message > isn't edited all the commits that are picked have the same committer > date. If a commit is reworded it's committer date will be a later time > as it is created by running an separate instance of 'git commit'. If > the reworded commit is follow by further picks, those later commits > will have an earlier committer date than the reworded one. This is > caused by git caching the default date used when GIT_COMMITTER_DATE is > not set. Reset the cached date before a commit is generated > in-process. > > Signed-off-by: Johannes Sixt <j6t@kdbg.org> > --- > sequencer.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/sequencer.c b/sequencer.c > index f9d1001dee..f0bac903a0 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -1148,6 +1148,8 @@ static int try_to_commit(struct strbuf *msg, const char *author, > goto out; > } > > + reset_ident_date(); > + > if (commit_tree_extended(msg->buf, msg->len, &tree, parents, > oid, author, opts->gpg_sign, extra)) { > res = error(_("failed to write commit object")); ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH] ident: don't cache default date 2018-04-18 18:15 ` Johannes Sixt 2018-04-18 21:15 ` Junio C Hamano @ 2018-04-19 9:15 ` Phillip Wood 2018-04-20 8:11 ` Johannes Schindelin 1 sibling, 1 reply; 17+ messages in thread From: Phillip Wood @ 2018-04-19 9:15 UTC (permalink / raw) To: Johannes Sixt, phillip.wood Cc: Ævar Arnfjörð Bjarmason, Johannes Schindelin, Junio C Hamano, Git Mailing List On 18/04/18 19:15, Johannes Sixt wrote: > Am 18.04.2018 um 19:47 schrieb Phillip Wood: >> On 18/04/18 12:27, Ævar Arnfjörð Bjarmason wrote: >>> On Wed, Apr 18 2018, Phillip Wood wrote: >>>> From: Phillip Wood <phillip.wood@dunelm.org.uk> >>>> as it is created by running an separate instance of 'git commit'. If >>>> the reworded commit is follow by further picks, those later commits >>>> will have an earlier committer date than the reworded one. This is >>>> caused by git caching the default date used when GIT_COMMITTER_DATE is >>>> not set. Fix this by not caching the date. >>>> >>>> Users expect commits to have the same author and committer dates when >>>> the don't explicitly set them. As the date is now updated each time >>>> git_author_info() or git_committer_info() is run it is possible to end >>>> up with different author and committer dates. Fix this for >>>> 'commit-tree', 'notes' and 'merge' by using a single date in >>>> commit_tree_extended() and passing it explicitly to the new functions >>>> git_author_info_with_date() and git_committer_info_with_date() when >>>> neither the author date nor the committer date are explicitly >>>> set. 'commit' always passes the author date to commit_tree_extended() >>>> and relied on the date caching to have the same committer and author >>>> dates when neither was specified. Fix this by setting >>>> GIT_COMMITTER_DATE to be the same as the author date passed to >>>> commit_tree_extended(). >>>> >>>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> >>>> Reported-by: Johannes Sixt <j6t@kdbg.org> >>>> --- >>>> >>>> I'm slightly nervous that setting GIT_COMMITTER_DATE in >>>> builtin/commit.c will break someone's hook script. Maybe it would be >>>> better to add a committer parameter to commit_tree() and >>>> commit_tree_extended(). > > While I like the basic theme of your patch, I think we should fix this > case in a much simpler way, namely, use the infrastructure that was > introduced for git-am. > > I've shamelessly lifted the commit message from your patch. Thanks, that is a better way (I'm annoyed with myself for not having noticed reset_ident_date() when I edited the function above it) Best Wishes Phillip > ---- 8< ---- > Subject: [PATCH] sequencer: reset the committer date before commits > > Now that the sequencer commits without forking when the commit message > isn't edited all the commits that are picked have the same committer > date. If a commit is reworded it's committer date will be a later time > as it is created by running an separate instance of 'git commit'. If > the reworded commit is follow by further picks, those later commits > will have an earlier committer date than the reworded one. This is > caused by git caching the default date used when GIT_COMMITTER_DATE is > not set. Reset the cached date before a commit is generated > in-process. > > Signed-off-by: Johannes Sixt <j6t@kdbg.org> > --- > sequencer.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/sequencer.c b/sequencer.c > index f9d1001dee..f0bac903a0 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -1148,6 +1148,8 @@ static int try_to_commit(struct strbuf *msg, const char *author, > goto out; > } > > + reset_ident_date(); > + > if (commit_tree_extended(msg->buf, msg->len, &tree, parents, > oid, author, opts->gpg_sign, extra)) { > res = error(_("failed to write commit object")); > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH] ident: don't cache default date 2018-04-19 9:15 ` Phillip Wood @ 2018-04-20 8:11 ` Johannes Schindelin 2018-04-20 9:41 ` Phillip Wood 0 siblings, 1 reply; 17+ messages in thread From: Johannes Schindelin @ 2018-04-20 8:11 UTC (permalink / raw) To: phillip.wood Cc: Johannes Sixt, Ævar Arnfjörð Bjarmason, Junio C Hamano, Git Mailing List [-- Attachment #1: Type: text/plain, Size: 2715 bytes --] Hi Phillip, On Thu, 19 Apr 2018, Phillip Wood wrote: > On 18/04/18 19:15, Johannes Sixt wrote: > > Am 18.04.2018 um 19:47 schrieb Phillip Wood: > >> On 18/04/18 12:27, Ævar Arnfjörð Bjarmason wrote: > >>> On Wed, Apr 18 2018, Phillip Wood wrote: > >>>> From: Phillip Wood <phillip.wood@dunelm.org.uk> > >>>> as it is created by running an separate instance of 'git commit'. If > >>>> the reworded commit is follow by further picks, those later commits > >>>> will have an earlier committer date than the reworded one. This is > >>>> caused by git caching the default date used when GIT_COMMITTER_DATE is > >>>> not set. Fix this by not caching the date. > >>>> > >>>> Users expect commits to have the same author and committer dates when > >>>> the don't explicitly set them. As the date is now updated each time > >>>> git_author_info() or git_committer_info() is run it is possible to end > >>>> up with different author and committer dates. Fix this for > >>>> 'commit-tree', 'notes' and 'merge' by using a single date in > >>>> commit_tree_extended() and passing it explicitly to the new functions > >>>> git_author_info_with_date() and git_committer_info_with_date() when > >>>> neither the author date nor the committer date are explicitly > >>>> set. 'commit' always passes the author date to commit_tree_extended() > >>>> and relied on the date caching to have the same committer and author > >>>> dates when neither was specified. Fix this by setting > >>>> GIT_COMMITTER_DATE to be the same as the author date passed to > >>>> commit_tree_extended(). > >>>> > >>>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > >>>> Reported-by: Johannes Sixt <j6t@kdbg.org> > >>>> --- > >>>> > >>>> I'm slightly nervous that setting GIT_COMMITTER_DATE in > >>>> builtin/commit.c will break someone's hook script. Maybe it would be > >>>> better to add a committer parameter to commit_tree() and > >>>> commit_tree_extended(). > > > > While I like the basic theme of your patch, I think we should fix this > > case in a much simpler way, namely, use the infrastructure that was > > introduced for git-am. > > > > I've shamelessly lifted the commit message from your patch. > > Thanks, that is a better way (I'm annoyed with myself for not having > noticed reset_ident_date() when I edited the function above it) Don't be too annoyed. I did remember that The Linus had complained about something similar and assumed that it had been fixed in the meantime, but I failed to find it within 30 minutes where I tried to dig through public-inbox and pu. Thanks Hannes for remembering, and for coming up with the final form of the patch! Ciao, Dscho ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH] ident: don't cache default date 2018-04-20 8:11 ` Johannes Schindelin @ 2018-04-20 9:41 ` Phillip Wood 0 siblings, 0 replies; 17+ messages in thread From: Phillip Wood @ 2018-04-20 9:41 UTC (permalink / raw) To: Johannes Schindelin, phillip.wood Cc: Johannes Sixt, Ævar Arnfjörð Bjarmason, Junio C Hamano, Git Mailing List On 20/04/18 09:11, Johannes Schindelin wrote: > Hi Phillip, > > On Thu, 19 Apr 2018, Phillip Wood wrote: > >> On 18/04/18 19:15, Johannes Sixt wrote: >>> Am 18.04.2018 um 19:47 schrieb Phillip Wood: >>>> On 18/04/18 12:27, Ævar Arnfjörð Bjarmason wrote: >>>>> On Wed, Apr 18 2018, Phillip Wood wrote: >>>>>> From: Phillip Wood <phillip.wood@dunelm.org.uk> >>>>>> as it is created by running an separate instance of 'git commit'. If >>>>>> the reworded commit is follow by further picks, those later commits >>>>>> will have an earlier committer date than the reworded one. This is >>>>>> caused by git caching the default date used when GIT_COMMITTER_DATE is >>>>>> not set. Fix this by not caching the date. >>>>>> >>>>>> Users expect commits to have the same author and committer dates when >>>>>> the don't explicitly set them. As the date is now updated each time >>>>>> git_author_info() or git_committer_info() is run it is possible to end >>>>>> up with different author and committer dates. Fix this for >>>>>> 'commit-tree', 'notes' and 'merge' by using a single date in >>>>>> commit_tree_extended() and passing it explicitly to the new functions >>>>>> git_author_info_with_date() and git_committer_info_with_date() when >>>>>> neither the author date nor the committer date are explicitly >>>>>> set. 'commit' always passes the author date to commit_tree_extended() >>>>>> and relied on the date caching to have the same committer and author >>>>>> dates when neither was specified. Fix this by setting >>>>>> GIT_COMMITTER_DATE to be the same as the author date passed to >>>>>> commit_tree_extended(). >>>>>> >>>>>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> >>>>>> Reported-by: Johannes Sixt <j6t@kdbg.org> >>>>>> --- >>>>>> >>>>>> I'm slightly nervous that setting GIT_COMMITTER_DATE in >>>>>> builtin/commit.c will break someone's hook script. Maybe it would be >>>>>> better to add a committer parameter to commit_tree() and >>>>>> commit_tree_extended(). >>> >>> While I like the basic theme of your patch, I think we should fix this >>> case in a much simpler way, namely, use the infrastructure that was >>> introduced for git-am. >>> >>> I've shamelessly lifted the commit message from your patch. >> >> Thanks, that is a better way (I'm annoyed with myself for not having >> noticed reset_ident_date() when I edited the function above it) > > Don't be too annoyed. I did remember that The Linus had complained about > something similar and assumed that it had been fixed in the meantime, but > I failed to find it within 30 minutes where I tried to dig through > public-inbox and pu. Thanks, that makes we feel better about it (thanks for taking the time to try and find the original mail as well) Phillip > Thanks Hannes for remembering, and for coming up with the final form of > the patch! > > Ciao, > Dscho > ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-04-20 9:41 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-13 16:52 Bug: rebase -i creates committer time inversions on 'reword' Johannes Sixt 2018-04-14 11:15 ` Phillip Wood 2018-04-14 13:11 ` Johannes Schindelin 2018-04-16 9:48 ` Phillip Wood 2018-04-19 9:17 ` Phillip Wood 2018-04-15 21:35 ` Junio C Hamano 2018-04-16 5:56 ` Johannes Sixt 2018-04-17 1:37 ` Junio C Hamano 2018-04-18 10:19 ` Phillip Wood 2018-04-18 10:22 ` [RFC PATCH] ident: don't cache default date Phillip Wood 2018-04-18 11:27 ` Ævar Arnfjörð Bjarmason 2018-04-18 17:47 ` Phillip Wood 2018-04-18 18:15 ` Johannes Sixt 2018-04-18 21:15 ` Junio C Hamano 2018-04-19 9:15 ` Phillip Wood 2018-04-20 8:11 ` Johannes Schindelin 2018-04-20 9:41 ` Phillip Wood
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).