Bug#814852: RFS: openfst/1.5.1-1 -- weighted finite-state transducers library
Hi Jakub,
On 08/03/2016 19:00, Jakub Wilk wrote:
> * 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.
I rewrote that phrase so that now it is, hopefully, more clear.
>> 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?
Probably.
The situation seems much much better than before (I did not experience test failures anymore), but, according to the author, it still requires testing.
> Typo in the patch header:
> idemppotent -> idempotent
Fixed.
> 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 author of the patch just reproduced what he found on the code.
In the same code "Hopcroft" is spelled "HopCroft", "Hopcroft", "hopcroft", so I think the correct spelling is "Hopcroft".
However I would prefer not to fix it until the patch is known to be stable.
>>> 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)
And I also made this mistake two times in a single package... :-/
> Sounds good enough for me. Let's not overthink this. :)
Perfect!
>> 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
So we may have a failure there. :-/
> 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.
You are right.
>>> 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
I tried to improve that sentence.
> 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
Added a patch for these.
Bests,
Giulio.
Reply to: