[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#793876: RFS: chrony/1.31.1-1



Hi Vincent,

[I am merging the e-mails you send yesterday to make the thread easier
again.]

On 20-08-15 18:12, Vincent Blut wrote:
> Le jeu. 20 août 2015 à 12:10, Paul Gevers <elbrus@debian.org> a écrit :
>> On 19-08-15 21:29, Vincent Blut wrote:
>> 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.

Ack.

>> 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.

Ack.

>> 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.

I didn't check if ntp is also doing the conflicts/replaces/provides
dance on time-deamon. If so I than you don't need to mention ntp
specifically at all¹¹.

> Anyway, depending on your answer, I’ll revert this commit.

Lets not do that until we agree. :)

>> 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?

Ack on the other mail of 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.

Oh, you not understanding me makes sense. It was me who didn't
understand what you were doing. I was assuming there now was a new
mechanism, but now you explained it is just a better implementation of
"the same thing". But then again, maybe make that a little clearer in
your changelog for others like me?

>> 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 haven't looked it up, is util-linux in essential? Otherwise, shouldn't
you depend on it with the higher than dependency?

>> 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.

Ack.

>> 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?

I am talking about "The scripts /etc/ppp/ip-up.d/chrony,
/etc/ppp/ip-down.d/chrony, and read key 1 from /etc/chrony/chrony.keys
and use it as the password to send chronyc commands." In my version that
is on line 36.

>> 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?

You confirm my ideas here. But as I mentioned in my other mail (below),
I challenge you to come up with a way to run them which is acceptable in
Debian's autopkgtest framework.

On 20-08-15 18:36, Vincent Blut wrote:
> Le jeu. 20 août 2015 à 13:54, Paul Gevers <elbrus@debian.org> a écrit
>> Is there a reason why you don't install the examples? (I could
>> imaging it is because you setup the package in Debian anyways, but
>> please tell).
>
> Most of them can’t be applied as is due to not being Debianized, or 
> because preliminary packaging stuff need to be done, etc.

Even if they don't apply straight, unless all they do is already done by
the package they may be interesting as examples don't you think? (But
maybe then it is good to add a note saying that they don't work out of
the box).

>> It would be could if you investigated if you can run the upstream
>> tests. I can come up with multiple ways to do this, I like you to
>> at least give it a thought and maybe propose something (not
>> required for this upload though).
>
> See my answer on this front in the previous mail.

See my answer higher up, I still challenge you.

>> Have you been active in pursuing the other bugs as well?
>

> Definitely, I intend to mark some of them as /wontfix/, some will be
> closed because there are outdated and some will be fixed by providing
> the necessary features.

Ack.

> By the way, if I want to close these outdated bug reports, what’s the
> canonical way to do it? I guess I can’t do that from d/changelog?

See my other mail. And no, don't close bugs in the changelog if they are
not closed by the upload.

Paul

¹¹ https://www.debian.org/doc/debian-policy/ch-relationships.html#s7.6.2

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: