[Bug 754245] Review Request: ocaml-menhir - LR(1) parser generator for OCaml

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=754245

--- Comment #1 from Scott Tsai <scottt.tw at gmail.com> 2011-12-16 19:37:39 EST ---
I have some problems with how menhir is split into three packages:

1. /usr/bin/manhir and /usr/share/menhir/standard.mly should be in the same
package.

>From the menhir manual:
"5.1 Splitting specifications over multiple files
This mechanism is, in fact, how Menhir’s standard library (§5.4) is made
available: even though its name does not appear on the command line, it is
automatically joined with the user’s explicitly-provided grammar
specifications, making the standard library’s definitions globally visible."

See also section 5.4 "The standard library" and the --no-stdlib and --stdlib
options.
This is also how Debian does it:
http://packages.debian.org/sid/amd64/menhir/filelist

2. Note that compiling code generated with "menhir --table" requires menhirLib
>From the menhir Manual:
"When --table is used ... These tables are not quite stand-alone: they are
exploited by an interpreter, which is shipped as part of the support library
MenhirLib. For this reason, when --table is used, MenhirLib must be made
visible to the Objective Caml compilers, and must be linked into your
executable program. The --suggest-* switches, described above, help do this."

I can see how separating "files required to run the parser generator" from
"files required to compile the generated parser source" can make sense similar
to how some projects ships C files generated from bison but then does
separating ocaml-menhir from ocaml-menhir-devel make sense?

3. The Fedora OCaml packaging guidlines
(http://fedoraproject.org/wiki/Packaging:OCaml) suggest that:
*.cmi should be in the main package (you're placing it in -devel)
*.o should be in the -devel package (you're placing it in main)
Why?

4. The "demos" code you placed in -devel requires the "menhir" binary and
should be in the same package. (It's also what Debian does)
You also didn't remove the generated .ml (and .mli and .mli.d derived from
them).

Try "make clean" under the demos/ directory.

5. For the "demos" directory, I have a patch that will make it build outside of
the menhir source tree. This makes the example code easier to use for the
Fedora user.

6. Stylistically, I suggest your use %{_buildroot} instead of $RPM_BUILD_ROOT.
Since you're relying new rpmbuild features like "no defattr" "no %clean
section" anyway.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.