Sandro Tosi wrote:
> Hello Bob,
> here's a brief review of the package.
Thank you for reviewing the package!
> debian/changelog
> - don't rewrite history, so please restore the old changelog entries,
> even if they have a weird "Closes=xxx" in the first entry line
The two entries are:
time (1.7-4) unstable; urgency=low, Closes=31767
* debian/{rules,postinst,postrm}: Removed support for html documentation
through menu as it is already provided by doc-base (fixes #31767)
-- Dirk Eddelbuettel <edd@debian.org> Thu, 14 Jan 1999 20:42:16 -0500
time (1.7-3) unstable; urgency=low, Closes=31164
* Upgraded to Debian Policy 2.5.0.0 (no change)
* Added doc-base support (fixes #31164)
-- Dirk Eddelbuettel <edd@debian.org> Tue, 5 Jan 1999 20:43:41 -0500
I fixed those entries because otherwise lintian complains with these
warnings:
W: time: syntax-error-in-debian-changelog line 175 "unknown key-value key Closes - copying to XS-Closes"
W: time: syntax-error-in-debian-changelog line 182 "unknown key-value key Closes - copying to XS-Closes"
Plus the changelog entries for each of them already mention that they
fix those particular bugs so no historical information is lost by
updating the syntax. Also there is a long history in Debian of
correcting changelog entries when appropriate and this certainly seems
like an appropriate instance if any.
I think these should be fixed. However I will reintroduce those
lintian warnings since you feel strongly about it.
> debian/control
> - why didn't you bump debhelper to 9, which is the latest version?
> just to undestand if there was some reason
Because 8 was the latest version when I built the package. Version 9
was uploaded subsequently on 2012-01-15. I will rebuild it with
version 9 now that it is the latest. The compat level of the
previous package was 4.
> - I personally would have left 'GNU time' in the short description line
I changed the line due to input from other people concerning the
"correct" format for the short description. Plus it gave me this
lintian warning so I needed to change it.
W: time: description-synopsis-starts-with-article
But I am happy to modify it again. I don't feel strongly about it.
I would have left "cpu" lower case too. It has long ago moved from
being an acronym to being an accepted word of its own. Just like
"email" and others. But there was Bug#492669 from Filipus Klutiero
who is a good soul. It seemed magnanimous to simply make the change
than to argue such a minor point.
> debian/copyright
> - you misses to state the previous maintainer(s) copyright. While this
> is non necessary for teh upload, is kinda rude ;) please add at least
> the entry for Tollef (easily gettable from the start to the end of his
> maintainership of the package).
If you noticed the changelog entry then you know that I was not being
rude to Tollef or any of the other contributors.
Also I think you meant Dirk Eddelbuettel not Tollef. Tollef didn't
list himself as owning copyright for any of the files in the package.
His name did not appear in the previous copyright file at all. It was
Dirk Eddelbuettel who is credited with having created the package
originally. Although the name of Peter Tobias appears earlier in the
changelog file.
I believe the copyright file should list the _current_ status of the
files of the package. The changelog is the appropriate place for
historical information. So on a technical point: How would Tollef be
credited in the new DEP5 copyright format? I am not yet comfortable
with the new format even after studying the documentation on it in
great detail. Is there a section for listing emeritus maintainers?
However because of looking into this now I do see that the time.1 file
is copyright by Dirk Eddelbuettel. Missing that file for the
copyright listing was definitely an oversight on my part. If I had
caught that I would defintely have listed it. Note that it was not
listed as such in the previous version of the copyright file.
I will update the copyright file for the new version to note that
file's copyright status. Which uncomfortably does not use a standard
license. It says only:
Copyright (C) Dirk Eddelbuettel but freely redistributable
And therefore I can only infer the following for the new DEP5
copyright file format:
Files: debian/time.1
Copyright: Copyright 1996 Dirk Eddelbuettel
<edd@miles.econ.queensu.ca>
License: freely redistributable
Copyright Dirk Eddelbuettel but freely redistributable
The one thing that I really wish people would learn would be to avoid
creating new and unique licenses. It is almost always better to use
one of the standard existing licenses than to create a new one.
However this was from 1996 more than 16 years ago. We learn things as
time goes by.
Also I see that the doc-base file was created by Dirk Eddelbuettel but
has no license associated with it. It is a small file with only six
lines of unique content and I think safe to assume that he intended to
apply the same license to it as he applied to the time.1 file. For
the moment I will apply the same copyright information to it too.
Note that I wrote the current debian/rules file fresh based upon other
debhelper examples. The previous rules file used cdbs. So the new
file is completely different from the previous.
> debian/time.1
> - did you consider pushing the manpage upstream?
Due to the current circumstances of the current upstream being
moribund I did not. There hasn't been an upstream release since
1996. However it would certainly be desirable.
Also since for GNU packages the primary documentation format is
texinfo format there isn't motivation for GNU packages to include man
pages. Most GNU packages do however include man pages. But most of
those that do use help2man to produce them from the online --help
output. So I think it is unlikely that the current time.1 file would
be incorporated upstream even when upstream finally wakes up from its
long sleep.
> debian/source/format
> - is there some reason not to move to the "3.0 (quilt)" format?
There is only so much that one can do in one sitting. I had to stop
somewhere. Also the first potential sponsor for the package wasn't a
fan of quilt. And personally I don't like it very much either. It is
one of those tools that you either love because you are already using
quilt and therefore know how to use it efficiently or you hate because
you aren't using it and don't know how to drive it and it is
automatically doing things that cause trouble during the package
build. I am in the latter camp. I have used it before but find it a
pain. But if you want it in quilt format I will move it there.
> I also see that there are several changes performed directly in the
> upstream code, such as autotools/configure/make and friends,
Yes. But note that those had been made before too. I only updated
the autotools files (again) because the current ones were quite aged
and dearly in need of being updated. Those were not "patched" in the
sense of editing the file with changes but updated whole by using the
current autotools to create new versions of those files from the
included source files. (The source being the configure.ac and
Makefile.am files.)
> .info file and so on.
The info documentation files and source files were however actually
patched.
> It would be clearer if they were separate patches living in the
> debian/patches directory, so to clearly identify why a change has
> been made, and possible traking its upstream merge.
Sigh. I will convert the package to quilt format.
> They are quite easy to fix, so the faster you reply and prepare a new
> pkg, the quicker I'll upload :)
Splitting out the patches and converting to quilt is not trivial. But
just the same I know that is the most popular current format and will
make that change.
> Addenda, taken from lintian output after build:
>
> I: time source: debian-watch-file-is-missing
> is it possible to add it? does it make sense for a GNU project?
Up to date lintian does not give me that message.
Now running lintian...
Finished running lintian.
What version of lintian are you using? I am using version 2.5.9 which
is up to date in Sid as of today.
It is easy to add a watch file. But note that the upstream release
area has not updated since 11-Jul-1996 so this particular feature is
of little value at this moment. But perhaps at some point in the
future it would have value.
> W: time: hardening-no-fortify-functions usr/bin/time
> did you consider enable the hardening flags?
That is a brand new lintian warning and did not exist when I built the
package. So the answer is no that I did not consider it. It wasn't
an item to be considered at that time. I am skeptical that time would
gain value by using them. But I am not opposed to adding them.
However I am using the debhelper build system. Shouldn't it be doing
this automatically there? Isn't that the purpose of using such a
build system so that standardized behaviors are implemented uniformly
across everything all at once?
How does one enable hardening flags when using the dh build system?
> P: time: no-homepage-field
> can you please add it?
Up to date lintian does not give me that message. But that field is
easy to add. I will add that field to the control file.
> I: time: hyphen-used-as-minus-sign usr/share/man/man1/time.1.gz:99
> I: time: hyphen-used-as-minus-sign usr/share/man/man1/time.1.gz:135
> I: time: hyphen-used-as-minus-sign usr/share/man/man1/time.1.gz:136
> I: time: hyphen-used-as-minus-sign usr/share/man/man1/time.1.gz:245
> I: time: hyphen-used-as-minus-sign usr/share/man/man1/time.1.gz:247
> I: time: hyphen-used-as-minus-sign usr/share/man/man1/time.1.gz:254
> it would be nice if the manpage would be fixed with them too
I do not see these messages. Lintian is clean for me. I will need to
research what is happening there.
After looking into it I am not sure I agree with that lintian warning.
I think it is wanting to use a long dash instead of a minus. But I
may have silenced it just the same. Since I do not receive that
lintian warning I cannot check to see if I have silenced it.
As soon as I get the build happy as a quilt format package I will post
an update.
Thank you again for reviewing the package!
Bob
Attachment:
signature.asc
Description: Digital signature