Le jeu. 20 août 2015 à 12:10, Paul Gevers <elb...@debian.org> a
écrit :
Hi Vincent,
Hello Paul,
Live from Debconf15.
I’m watching you folks, you all look great. :-)
On 19-08-15 21:29, Vincent Blut wrote:
I am looking for a sponsor for my package "chrony"
Please note this is a first manual inspection. Not all items are
critical, most are just nitpicks or tips or questions.
Please add the CVE numbers that were fixed by upstream to your
changelog
such that the trackers can find it automatically. TIP: if you would
have
done that and mention that in your RFS you would have probably found a
sponsor earlier.
I didn’t include them because those fixes have been backported to
chrony
1.30-2 in Debian (thanks Joachim btw) and consequently the CVE numbers
have
already been mentionned in this release’s changelog.
Your priority switch from extra to optional may require a ping to
somebody. I am not sure and I would need to search, so please do that
yourself.
Yes. I’ll have to send a bug report to the ftp.debian.org
pseudo-package
asking for the modification of the section/priority in the
"override-file".
Will do later today… or tomorrow.
Which file do you have in common with ntp? Please re-read ¹.
I guess I’ve been misled by § 7.6.2! The previous section shows the
usage
of the 'replaces:' field for packages providing *files* already provided
by another package. However, the section 7.6.2 seems to be for
*packages* that
/do conflict/; I interpreted that /do conflict/ by "packages providing
the same
functionality". I even was quite sure my interpretation was correct
after seeing
the usage example about MTAs.
Anyway, depending on your answer, I’ll revert this commit.
I assume that the change of maintainership has the consent of Joachim?
Yes, we’ve discussed about this privately some times ago. Still ok
Joachim?
Wouldn't the hwclockfile stuff in /etc not warrant an debian/NEWS
update? Or at the very least some help in the changelog?
I don’t think so. Finally, that change have no impact for the user.
Previously we had to check (in the postinst script) if the RTC keeps
local time
or UTC by parsing /etc/adjtime and/or /etc/default/rcS. Depending on
the result,
we set (or no) the "rtconutc" directive in /etc/chrony.conf.
But now chrony is grown enough to check that by itself. Each time it is
started,
it will parse the correspondent value in the /etc/adjtime file.
So, as you can see, the whole point of using the hwclockfile directive
is to have
something cleaner than playing the text processing game for the same
result.
Doesn't this
actually require a migration path? What if the /etc/chrony and
/etc/adjtime are NOT answering the same?
Well, I’m not sure I’m understanding you here. The chronyd daemon
will use what
/etc/adjtime returns, thanks to the 'hwclockfile /etc/adjtime'
directive.
Can you please explain me how commit 1ce86d3 works (the Breaks of
util-linux).
As the hwclockfile directive can only deal with /etc/adjtime, we need
to ensure
that we migrated from /etc/default/rcS to /etc/adjtime. To be honest,
I’m not
sure that break is even needed, because this migration happened prior
to Wheezy.
I assume you tested all migrations for admins that already ran chrony
as
a different users as described in the README.Debian. Are the manual
steps even needed? Shouldn't this go into a NEWS file instead of the
README file?
I tested a lot of use cases, but Jerome Benoit informed me he had an
issue possibly
related to this change, but as he uses a custom init script etc., I
will have to
check his atypical configuration.
Line 36 of the README.Debian file ends weird now, you removed a
filename
but not the "and" in front.
Indeed, will fix. You mean line 27 right?
Nice to have, could you think of some autopkgtest test²? And why are
the
tests disabled. Unless they fail and can't be fixed, it is really
recommended to run them.
I’m definitely interested in autopkgtest. However I’ll need some
times to dive
through its meanders.
I don’t known why tests have originally been disabled, but I guess
it’s because
they depend on the clknetsim tool which is not packaged in Debian.
Also, if that
tool isn’t installed on the system, the "test.common" script will try
to download
and build the tool, which is a quite invasive method. Am I wrong
Joachim?
I think the comments you added in commit df80cd25 in the copyright
file,
should the "Comment" field.³
Definitely, will fix.
And tip to prevent the fix in commit 7245a4, use dch to write the
timestamps (e.g. dch -rm)
So true. I discovered this tool to late. ;-)
Thanks for the reminder.
You could maybe remind upstream to update their copyright years when
they make changes.
Indeed.
Paul
Thanks for the initial review,
Vincent