Bug#813096: AW: Bug#813096: ITP: logdata-anomaly-miner -- lightweight tool for log checking, log analysis

[Fiedler Roman]

> Von: Gianfranco Costamagna [mailto:locutusofb...@debian.org]
> [Snip]
>> Ah, I see. I assumed that the patch editor would cut the lower part in same 
>> fashion as git/svn would do. Fixed.
>
> nack
> .
> logdata-anomaly-miner (0.0.2-1) unstable; urgency=low
> .
> * Initial inclusion of logdata-anomaly-miner to Debian
> (Closes: #813096)
> Author: Roman Fiedler 
> Bug-Debian: https://bugs.debian.org/813096
>
> this seems useless ^^^
> you need to close a specific bug, not the itp one
> (or just delete the lines if there isn't a debian bug corresponding
> to the patch)

Understood for the ITP-case. (I assumed "dpkg-source --commit" would have 
added those lines for a reason, so I kept them). As there is no specific bug 
to close, I removed all the lines mentioned above EXCEPT the "Author" one, as 
this should be mandated by "dep3", if understood correctly.

What would be the correct way of handling for non-ITP bugs? Should they be 
only mentioned in the changelog, the patches or both? Some example what I 
could think about: "Backport of some-logic-flaw from upstream". Does Debian 
system also use the bug reference entries from patches to auto-close bugs? I 
assumed, that only changelog-bug references would do that?

> [Snip]
> I guess we are mostly ok!

Seems so. As the upstream native package would also greatly benefit from these 
changes, I will backport most of the stuff done here for the next version, 
then we could drop most or perhaps even all of the patches. Would it make 
sense to keep the sponsor-workflow also for those uploads (pro: learn from 
sponsor, con: consumes sponsor's time) or becoming a maintainer (pro: shorter 
upload workflow, con: no second opinion on own work)?

Thanks for your patience!


smime.p7s
Description: S/MIME cryptographic signature

Bug#813096: AW: Bug#813096: ITP: logdata-anomaly-miner -- lightweight tool for log checking, log analysis

[Fiedler Roman]

> Von: Piotr Ożarowski [mailto:pi...@debian.org]
> 
> [Fiedler Roman, 2016-06-09]
> > > is AMiner and AMinerRemoteControl symlinked in /usr/bin/?
> > > (you can use debian/logdata-anomaly-miner.links to do that)
> >
> > Currently they are not symlinked. I took the same approach as with
> > e.g. /usr/lib/apt/methods/http or /usr/lib/eject/dmcrypt-get-device
> > where those binaries are usually not intended to be called by user
> > directly. Meanwhile this has changed, e.g. AMinerRemoteControl is more
> > or less now OK for cmd-line use. So I would add the symlinks. Or would
> > moving of the files be more Debianic?
> 
> dh_python2 doesn't add interpreter to Depends for scripts that are not
> in $PATH. That's why lintian complains. You can:
> * symlink them in /usr/bin/ (or any other appropriate path)

I did that, modification:

$ cat debian/logdata-anomaly-miner.links 
/usr/lib/logdata-anomaly-miner/AMiner /usr/bin/AMiner
/usr/lib/logdata-anomaly-miner/AMinerRemoteControl /usr/bin/AMinerRemoteControl

> * drop executable bit from these scripts, if they're not meant to be
>   execute  (IIRC lintian ignores shebangs in non-executable files)

not an option

> * or if they're meant to be executed, but you don't want them to be
>   available out of the box, add "python" to Suggests or Recommends

There are some usecases for users to call them, so let's have them available 
also.

Still lintian will complain about this file, current header:

#!/usr/bin/python2 -BEsSt

But as patching that to 2.7 will make the warnings go away, from my point of 
view, that workaround is sufficient for me.

New package is uploaded.

Are there any other open points I shall address?


smime.p7s
Description: S/MIME cryptographic signature

Bug#813096: AW: Bug#813096: ITP: logdata-anomaly-miner -- lightweight tool for log checking, log analysis

[Fiedler Roman]

> Von: Piotr Ożarowski [mailto:pi...@debian.org]
> 
> just a quick reply:
> 
> * private dir is the right call, do not install into dist-packages
>   (only "python-*" binary packages should install there)

OK, done.

> * install into /usr/lib/logdata-anomaly-miner/ and dh_python2 will pick
>   it up without any overrides, additional options, etc.

So I guess, before that (where dh_python2 refused to operate at all), using 
/usr/lib/aminer as a shorthand - as the package name is quite long - was a bad 
idea.

As import names cannot contain a dash, is use of 
"usr/lib/logdata-anomaly-miner/aminer" as module storage directory and "aminer" 
as shorthand module name in imports OK?

> * use dh --with python2 or dh_python2 (/usr/bin/dh_python2) but never
>   ever depend on internal implementation detail. I will change
>   /usr/share/dh-python/dh_python2 file name just to mess with you ;-P

OK. Understood. Using the "usr/lib/logdata-anomaly-miner" layout for python 
stuff as recommended above, dh_python2 is more happy with the situation: It 
would complain about "python:Versions" substitution in

./debian/control:XB-Python-Version: ${python:Versions}

but XB-.. should be deprecated anyway. I dropped also " XS-Python-Version" 
(which was hardcoded to >=2.6), as substitution would not work here also.

Still when building, lintian is unhappy with the output package:

E: logdata-anomaly-miner: python-script-but-no-python-dep 
usr/lib/logdata-anomaly-miner/AMiner
E: logdata-anomaly-miner: python-script-but-no-python-dep 
usr/lib/logdata-anomaly-miner/AMinerRemoteControl


Since the situation became a lot of clearer using your suggestions, I will 
again try to tackle the lintian issue.

Roman

> --
> Piotr Ożarowski Debian GNU/Linux Developer
> www.ozarowski.pl  www.griffith.cc   www.debian.org
> GPG Fingerprint: 1D2F A898 58DA AF62 1786 2DF7 AEF6 F1A2 A745 7645



smime.p7s
Description: S/MIME cryptographic signature

Bug#813096: AW: Bug#813096: ITP: logdata-anomaly-miner -- lightweight tool for log checking, log analysis

[Fiedler Roman]

> Von: Gianfranco Costamagna [mailto:locutusofb...@debian.org]
> Hi,
> 
> (answering where I can!)
>> Moving the code to "/usr/lib/python2.7/dist-packages/aminer" in fact allows
>> dh_python2 to extract the version information:
>>
>> Depends: python:any (<< 2.8), python:any (>= 2.7.5-5~), python-tz,
>> init-system-helpers (>= 1.18~)
>>
>> (Remark: Is there any reason to restrict the versions to >=2.7.5? The tools
>> should have compatibility with >=2.6 and I would expect the "Depends"
>> section
>> to somehow reflect that reality. Is DEBPYTHON_SUPPORTED and
>> DEBPYTHON_DEFAULT intended to be used to fix that?)
> 
> that stuff is built for unstable/testing, so the variables are filled
> with the current python situation
> (2.6 is only on old-stable, and Stretch has 2.7.11 already).
>
> Probably if you try to build it with a jessie chroot, you get different 
> values, and this is correct.
> (you can't in general install deb built against stretch into wheezy/jessie, 
> without
> breaking stuff, this is why some dependencies are not too relaxed, to avoid 
> people
> doing that, but instead upgrading the minimal set of packages to make sure
> things will work after the apt-pinning).
> 
> I don't foresee any issue here, because even in case of a backport, the
> package will need to be rebuilt on top of that.

OK, so I will not attempt to fiddle with that and stick to the stricter 
versions.

> >But doing the move, lintian will not like the produced package any more:
> >
> >E: logdata-anomaly-miner: python-script-but-no-python-dep 
> >usr/lib/aminer/AMiner
> 
> I can't say, I don't see the package installing stuff in usr/lib/python* on
> mentors

Maybe the hints from Piotr Ożarowski's followup mail will fix that, using his 
hints.

> >The rationale behind not putting aminer into dist-packages (and removing
> >dist-packages from python-path) was:
>> [Snip]
> 
> >b) prepare against possible future risks due to accidental loading (call to
> >__init__) of other packages residing in dist-packages, that may give rise to
> >privilege escalation (like the GNU-TLS CVE from this/last week)
> 
> mmm you want to avoid people importing your library?
>
> ok

Not directly avoid, but make them think more about it before adding. I also 
remember having issues with some svg-python library, that starts to misbehave 
as soon as other packages are installed because of "try: import xxx; ..." 
blocks. Just want to reduce the risks.
 
> >Of course, it should be possible to move the code to
> >/usr/lib/python2.7/dist-packages/aminer and perform the "link" operation,
> but
> >this could make it more "sexy" for an admin to include whole dist-packages
> in
> >python path again.
> >In that light, should the code be moved?
> 
> leaving to other folks the answer
> >Apart from that, this will also make it harder to use the same codebase for
> >both python2.6/python2.7, but that should be fixable by providing more
> >patches.
> 
> you can't use python2.6 on Stretch, so no issue here.

Understood, dropping the 2.6/2.7 compatibility stuff
 
> >> >-#!/usr/bin/python2 -BEsSt
> >> >+#!/usr/bin/python2.7 -BEsSt
> >>
> >> this should be also handled by dh_python2 AFAIK
> >
> >Even with move dh_python2 does not touch the files. The only difference is,
> >that lintian does not like the files any more.
> 
> can you please double check with the documentation?
> https://wiki.debian.org/Python/LibraryStyleGuide

The "python2" selection in the binaries was effect of reading it, but perhaps I 
got something wrong:

Intro: "It (the LibraryStyleGuide) is not intended to supplant the Debian 
Python Policy and in fact, if you have not read that document, stop now and go 
read it"

And from " Debian Python Policy"
https://www.debian.org/doc/packaging-manuals/python-policy/ch-python.html#s-interpreter_name

"Python scripts that require the default Python 2 version should specify 
python2 as the interpreter name."

So should be OK in my opinion.

> [Snip]


smime.p7s
Description: S/MIME cryptographic signature

Bug#813096: AW: Bug#813096: ITP: logdata-anomaly-miner -- lightweight tool for log checking, log analysis

[Gianfranco Costamagna]

Hi

> I still don't see "python" in build-dependencies.



>I tried that already, but somehow it did not work out (that's why the manual 
>"python2.6 | python2.7" is kept).

the reason should be the missing python dependency and the missing dh_python2 
call

>I'll stay on that one (see todos), at the moment I am trying to figure out 
>with strace what the rule
>
>/usr/share/dh-python/dh_python2 -p logdata-anomaly-miner 
>usr/lib/aminer/AMiner usr/lib/aminer/AMinerRemoteControl
>
>is doing and why it does not detect the correct version to pass it on to 
>dpkg-gencontrol.


don't know, dh --python2 should do the job.
not sure why you override that



>You are good! Funny, how dumb one can be ... Just used the stubs from previous 
>packages without thinking ...


:)

IIRC I did the same mistake some years ago :D


>Would that split of old postinst solve all problems without provoking new 
>ones?


I guess so
>a) new preinst: the user/creation (with #DEBHELPER#)
>b) postinst: chmod/chown, .. changes to files (WITHOUT the #DEBHELPER#)


with DEBHELPER too.

preinst <-- user creation
postinst <-- systemd start (automatically)


prerm <-- systemd stop (automatically)
postrm <-- user del and whatever

I'm not completely sure, just try, test and look at the policy
https://www.debian.org/doc/debian-policy/ch-maintainerscripts.html
>OK, I have deleted for now. What about the suggestion with the CHANGELOG?
>
>Currently debian/changelog is the "packaging changelog". But the software has 
>also a "feature/version changelog". Where should that be kept best? Options:


dh_installchangelogs is your friend :)
(but files that has similar names are added automatically I guess, just check if
it is added or not

>I have removed that part, both unused options and the error handling.


it should already be handled in the DEBHELPER macro, right?
>Thanks for the pointer to quilt. Seems to make sense, is on the todo list.

ok

>Understood, quilt will do here also.

yes
>OK, I will change. There is only one question open at the moment: what is the 
>Debian policy for versioning of those Debian-specific build files? Could they 
>be hosted just anywhere, e.g. adding them to the same upstream repository, 
>used to build the source.tar.gz or should the go to a separate repository, 
>perhaps even under control of Debian folks like the "Debian collab platform"? 
>Would https://wiki.debian.org/Alioth/PackagingProject make sense?


whenever you want, a "debian" branch in upstream packaging, a directory on your 
pc
whenever is convenient for you (also alioth, collab-maint, github)

having a different branch on git will make the packaging of new releases easy as
git checkout debian
git merge vVERSION-tag
dch
and so on
(YMMW)

>Thank you very much for your answers and patience, I will do a clean package 
>build after doing some more reading/testing.

keep your time :)

>Todos:
>* Fix: " dh --with=systemd,python2" somehow fails to substitute 
>dependencies/version in control
>* Check transition to "pybuild" and possible side effects (see mails 
>~2016-06-02) after solving the dependency problem
>* Enable quilt mode to patch upstream Launchpad source.tar.gz after deciding 
>about versioning of Debian-specific patches.


irc/OFTC and #debian-python/#debian-mentors channels might be your friends,
as well as manpages :)
(or the debian-mentors mail list)
G.

Bug#813096: AW: Bug#813096: ITP: logdata-anomaly-miner -- lightweight tool for log checking, log analysis

[Fiedler Roman]

> From: Gianfranco Costamagna [mailto:locutusofb...@debian.org]
>
> > E: logdata-anomaly-miner: python-script-but-no-python-dep
> >Tried to follow the guidelines, seems that everything works but lintian is 
> >still
> complaining. Changes recommended from various forums:
>
> >Any ideas would be appreciated. Otherwise I'll just add a workaround to get
> rid of the lintian error.
>
> I still don't see "python" in build-dependencies.
>
> BTW your rules hack is wrong, please just call dh --with=systemd,python2
>
> BTW "python2.6 | python2.7" <-- please remove dh_python2 should handle
> them too

I tried that already, but somehow it did not work out (that's why the manual 
"python2.6 | python2.7" is kept).
   dh_gencontrol
dpkg-gencontrol: warning: Depends field of package logdata-anomaly-miner: 
unknown substitution variable ${python:Depends}
dpkg-gencontrol: warning: File::FcntlLock not available; using flock which is 
not NFS-safe
dpkg-gencontrol: warning: package logdata-anomaly-miner: unknown substitution 
variable ${python:Versions}
   dh_md5sums
   dh_builddeb


I'll stay on that one (see todos), at the moment I am trying to figure out 
with strace what the rule

/usr/share/dh-python/dh_python2 -p logdata-anomaly-miner 
usr/lib/aminer/AMiner usr/lib/aminer/AMinerRemoteControl

is doing and why it does not detect the correct version to pass it on to 
dpkg-gencontrol.


> > W: logdata-anomaly-miner: maintainer-script-calls-systemctl prerm:30
> > W: logdata-anomaly-miner: maintainer-script-calls-systemctl prerm:31
>
> Gone by following changes  (BUT read below):
>
> >CAVEAT: Although lintian is happy, there are two issues with that change,
> that may/are problematic:
> >
> >a) I moved the DEBHELPER in prerm BEFORE my code to delete the users. Is
> the DEBHELPER code really intended to be run before or may this cause other
> issues?
>
> what about a "postrm" script?

You are good! Funny, how dumb one can be ... Just used the stubs from previous 
packages without thinking ...

> >b) In postinst DEBHELPER will automatically activate the systemd unit. But 
> >this
> was not done INTENTIONALLY! Therefore documentation included a section,
> what to >check/do before activating the service to avoid any risks. Are 
> there
> means to get to that state also with the dh-systemd components?
>
> preinst?

Would that split of old postinst solve all problems without provoking new 
ones?

a) new preinst: the user/creation (with #DEBHELPER#)
b) postinst: chmod/chown, .. changes to files (WITHOUT the #DEBHELPER#)

> >Thanks for the hint. If I understand correctly, pybuild will only care 
> >about
> maintaining the dependencies to the python packages and which interpreter
> to use but it >will NOT require the software to load from site-packages or 
> to
> create/use pyc files, which is currently avoided due to security reasons?
>
> I'm not sure :( sorry

No problem, kept as todo.

> >According to Debian manual, this should be used to automatically include
> files from uppermost directory of the unpacked source. Currently there are
> no useful files >at this location, all documentation is included directly 
> from
> source/root/usr/share/doc/aminer/. Would following files be sufficient?
>
> an empty file can be deleted, if you have no documentation, but having an
> empty file should be avoided
> (I don't like them, they overcomplicate packaging and are just useless)

OK, I have deleted for now. What about the suggestion with the CHANGELOG?

Currently debian/changelog is the "packaging changelog". But the software has 
also a "feature/version changelog". Where should that be kept best? Options:

* only upstream tar-ball but dropped during packaging
* in tar and package at same location
* in tar and package at different location

> [Snip]
>
> >Current content is "root/* ." There is no compiling involved, so this 
> >copies
> just the plain files into the package. Is there something else to be used 
> for this
> >purpose?
>
> not sure, does dh_systemd correctly handle the service file?

Yes, it seems so: from generated #DEBHEL:

# Automatically added by dh_systemd_enable
# This will only remove masks created by d-s-h on package removal.
deb-systemd-helper unmask aminer.service >/dev/null || true

# was-enabled defaults to true, so new installations run enable.
if deb-systemd-helper --quiet was-enabled aminer.service; then


> >How should that work? In generated DEBHELPER code I do not see any
> commands that would create the users? If you mean the length of " >abort-
> >upgrade|abort->remove|abort-deconfigure": this is from dh-make, should
> that be changed?
>
> yes, just override what you need, don't put default stuff there
> (and open a built-deb file, you will see the #DEBHELPER# macro substituted

I just kept it as it seemed to me to be a second line of defence against 
typos: everything not handled will end up in the "exit 1", so the first 
attempt to install the package during testing/continuous integration