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

Bug#814852: RFS: openfst/1.5.1-1 -- weighted finite-state transducers library



* Giulio Paci <giuliopaci@gmail.com>, 2016-03-07, 17:44:
I added a README.source

I don't like the phrase "build environment must not change during automated builds". It makes me think of that you must not install or remove packages when you package is building, which is technically true, but certainly not what you meant.

On Friday I discovered an issue with this patch that, randomly, prevents tests to complete.

I saw the patch was updated in f083eb6924bf. Is this fixed now?

Typo in the patch header:
idemppotent -> idempotent

Why is "HopCroft" spelled with capital C?
I've never seen this name spelled like that before.
(Searching for "HopCroft" in codesearch.d.n revealed that there's an embedded code copy of OpenFST in src:hfst. *sigh*)

I think the 500 MB/job limit is insufficient.
[...]
I increased the limit to 2Gb/job, but I am not completely convinced about this new limit.

s/Gb/GB/ (unless you meant gigabits :-P)

Sounds good enough for me. Let's not overthink this. :)

How likely is it that we are going to compile with parallel=2 on an amd64 system with 4Gb of RAM, without swap available?

Unlikely. Although we do seem to have an s390x buildd with only 3GB of RAM:
https://db.debian.org/machines.cgi?host=zemlinsky

I'd remove "Increase per job required memory to 2Gb for parallel builds.", because the previous version didn't have any limits, and you already said "Limit parallelism on buildds in order not to run out of RAM" earlier.

We have automatic debug packages these days, so I'd drop the -dbg package.
Dropped the -dbg package.

Hmm, "According to https://wiki.debian.org/AutomaticDebugPackages"; looks like truncated sentence. Anyway, IMO the announcement message is a better source of information on this: https://lists.debian.org/5675E791.6060705@thykier.net

cppcheck says:
[src/bin/fstcompile.cc:57]: (error) Memory leak: istrm
[src/bin/fstdraw.cc:72]: (error) Memory leak: ostrm
[src/bin/fstprint.cc:67]: (error) Memory leak: ostrm

--
Jakub Wilk


Reply to: