Bug 442266 - Review Request: embryo - Easy to use library for running compiled PAWN programs
Review Request: embryo - Easy to use library for running compiled PAWN programs
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Patrice Dumas
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 442437
  Show dependency treegraph
 
Reported: 2008-04-13 13:00 EDT by Pavel Shevchuk
Modified: 2008-05-06 13:37 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-05-06 13:37:38 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
pertusus: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Pavel Shevchuk 2008-04-13 13:00:48 EDT
Spec URL: http://rpm.scwlab.com/embryo-goes-rawhide/embryo.spec
SRPM URL: http://rpm.scwlab.com/embryo-goes-rawhide/embryo-0.9.1.042-1.fc9.src.rpm
Description:
Embryo is primarily a shared library that gives you an API to load and control
interpreted programs compiled into an abstract machine bytecode that it
understands.  This abstract (or virtual) machine is similar to a real machine
with a CPU, but it is emulated in software.  The architecture is simple and is
the same as the abstract machine (AMX) in the PAWN language (formerly called
SMALL) as it is based on exactly the same code. Embryo has modified the code
for the AMX extensively and has made it smaller and more portable.  It is VERY
small.  The total size of the virtual machine code AND header files is less
than 2500 lines of code.  It includes the floating point library support by
default as well.  This makes it one of the smallest interpreters around, and
thus makes is very efficient to use in code.


RPMLint is silent on all RPMs i built.
License is a bit customized, but Tom "spot" Callawayin from fedora-legal-list said it's MIT ( https://www.redhat.com/archives/fedora-legal-list/2008-April/msg00020.html )
I've been able to successfully use rpms built in mock on different machine to build software against this library.
Comment 1 Pavel Shevchuk 2008-04-13 13:22:39 EDT
Woops, forgot to include:
i386 and x86_64 RPMs built in mock: http://rpm.scwlab.com/embryo-goes-rawhide/
Comment 2 Pavel Shevchuk 2008-04-19 17:17:37 EDT
Improved spec.
New spec: http://rpm.scwlab.com/embryo-goes-rawhide/embryo.spec
New SRPM: http://rpm.scwlab.com/embryo-goes-rawhide/
embryo-0.9.1.042-2.fc9.src.rpm

* Sat Apr 19 2008 Pavel "Stalwart" Shevchuk <stlwrt@gmail.com> - 0.9.1.042-2
- Fixed timestamp of source tarball
- Preserve timestamps of installed files
- Added html docs
- Added missing dependencies for embryo-devel
- Added workaround for bug in embryo.pc. Proper fix is commited upstream
Comment 3 Pavel Shevchuk 2008-05-01 21:18:41 EDT
Fixed doxygen multilib issue. 

New spec: http://rpm.scwlab.com/embryo-goes-rawhide/embryo.spec
New SRPM: http://rpm.scwlab.com/fedora/e/9/embryo-0.9.1.042-3.x86_64/
embryo-0.9.1.042-3.fc9.src.rpm
Built RPMs: http://rpm.scwlab.com/fedora/e/9/
Comment 4 Patrice Dumas 2008-05-05 05:57:03 EDT
an incidental backtrace of embryo_cc -i

$ gdb --args embryo_cc -i
GNU gdb Fedora (6.8-1.fc9)
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i386-redhat-linux-gnu"...
(gdb) run
Starting program: /usr/bin/embryo_cc -i

Program received signal SIGSEGV, Segmentation fault.
0x0804ff26 in sc_compile (argc=2, argv=0xbfd15af4) at embryo_cc_sc1.c:631
631           if (!strcmp (argv[i], "-i") && *argv[i + 1])
(gdb) bt
#0  0x0804ff26 in sc_compile (argc=2, argv=0xbfd15af4) at embryo_cc_sc1.c:631
#1  0x08050b83 in main (argc=2, argv=0xbfd15af4, env=Cannot access memory at
address 0x35
) at embryo_cc_sc1.c:149
Comment 5 Patrice Dumas 2008-05-05 11:09:52 EDT
It seems to me that embryo_cc and the /usr/share/embryo/include/default.inc
file should be separated from the shared library. It seems to me that
it would be better in the devel package, and even better in a 
separate package. Either call it embryo and have the shared libs in 
embryo-libs, or put the shared libs in embryo and cann the package 
containing embryo_cc along embryo_cc or embryo-cc. I would prefer 
to have share libs in a -libs subpackage.
Comment 6 Pavel Shevchuk 2008-05-05 12:45:53 EDT
Embryo is for scripting existing applications, similar to LUA, and embryo 
scripts are usually distributed as source code. It's not the case of C 
compiler, such scripts are compiled and ran by regular users.

[stalwart@delta ~]$ rpm -ql lua.x86_64 | grep bin
/usr/bin/lua
/usr/bin/luac

LUA ships compiler in one package with libs.
Comment 7 Pavel Shevchuk 2008-05-05 16:49:04 EDT
* Mon May 05 2008 Pavel "Stalwart" Shevchuk <stlwrt@gmail.com> - 0.9.1.042-4
- Added workaround for segfault on incorrect arguments to embryo_cc

New spec: http://rpm.scwlab.com/embryo-goes-rawhide/embryo.spec
New SRPM: http://rpm.scwlab.com/fedora/e/9/embryo-0.9.1.042-4.fc9.x86_64/
embryo-0.9.1.042-4.fc9.src.rpm
Built RPMs: http://rpm.scwlab.com/fedora/e/9/
Comment 8 Patrice Dumas 2008-05-06 03:23:36 EDT
(In reply to comment #6)
> Embryo is for scripting existing applications, similar to LUA, and embryo 
> scripts are usually distributed as source code. It's not the case of C 
> compiler, such scripts are compiled and ran by regular users.

> LUA ships compiler in one package with libs.

Indeed, but in the case of embryo, it seems to be much more common
to use directly the libs. And e17 packages depending on embryo
only use the libs, so it seems to me that putting the interpreter in 
its own pacage (with name embryo) would be better, in order not to 
install the interpretr if not needed. Moreover the interpreter
(in contrast with the one embedded in the libs) seems to be less
well maintained (reading the README). Also, unless I am missing something
the %description is mostly for the -libs package, the interpreter has
still not been that much modified.

Now regarding the segfault, in my opinion it is much better to do a 
patch (and submit it upstream). A sed substitution will silently fail 
and in general is less readable. In the end it depends on the 
packager, but in my opinion the change in argv should be a patch, 
while the 2 other sed substitutions are right.

As a side note, this segfault is also an indication that the 
interpretor is not widely used.
Comment 9 Pavel Shevchuk 2008-05-06 05:31:38 EDT
lib executes code only in form of bytecode, and embryo_cc is the only
way to convert human-readable code to bytecode. I still insist on
having embryo_cc in default package.

Argument parser patch was upstreamed few minutes ago. I will remove
sed expression when new snapshot or 1.0.0 of embryo will be released
(e17 project leader said it's in RC stage already)
Comment 10 Patrice Dumas 2008-05-06 05:52:33 EDT
Ok, I just noticed that embryo_cc is used in one of the
edje programs, so looks like you are right. Also rereading
the library doc, it is clearly stated there. I'll review 
formally now.
Comment 11 Patrice Dumas 2008-05-06 05:56:39 EDT
* rpmlint is silent
* follow packaging guidelines
* pseudo MIT license included
* match upstream
d6f08bb2eb9c5c466eef7c1951d3f6ba  embryo-0.9.1.042.tar.bz2
* %files section right
* library correctly packaged

APPROVED
Comment 12 Patrice Dumas 2008-05-06 06:00:48 EDT
2 things I overlooked, though:

* The BuildRequires are better at the beginning
* A Requires for pkgconfig missing for -devel.
* I suggest usind the sed line to keep a footer like in other e* libraries
Comment 13 Pavel Shevchuk 2008-05-06 06:20:59 EDT
New Package CVS Request
=======================
Package Name: embryo
Short Description: Easy to use library for running compiled PAWN programs
Owners: stalwart
Branches: F-8 F-9
InitialCC:
Cvsextras Commits: yes
Comment 14 Kevin Fenzi 2008-05-06 12:19:00 EDT
cvs done.
Comment 15 Pavel Shevchuk 2008-05-06 13:37:38 EDT
Koji finished building package. Thanks to everyone!

Note You need to log in before you can comment on or make changes to this bug.