Bug 473037 (tinycc) - Review Request: tinycc - Tiny C Compiler
Summary: Review Request: tinycc - Tiny C Compiler
Keywords:
Status: CLOSED DEFERRED
Alias: tinycc
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Lubomir Rintel
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-11-26 08:15 UTC by Brennan Ashton
Modified: 2011-06-01 11:05 UTC (History)
13 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-05-08 07:12:53 UTC
Type: ---
Embargoed:
lkundrak: fedora-review?


Attachments (Terms of Use)

Description Brennan Ashton 2008-11-26 08:15:59 UTC
Spec URL: http://bashton.fedorapeople.org/tcc.spec
SRPM URL: http://bashton.fedorapeople.org/tcc-0.9.24-1.fc9.src.rpm
Description: 
I am not yet sponsored. 

Very small c compiler (about 100KB for x86 TCC executable, including C preprocessor, C compiler, assembler and linker). Also allows c files to be run as scripts ./acfile.c

I have a spec file that seems to work, but I was unsure about a lot of it. I am still not sure about the debug rpm.  I have tested the tcc bin and libs in mock with a few of the examples and they work fine.

I also have only been able to get it to compile on i386, it is a little hard as I only have koji to test other arch.

rpmlint is almost happy.

rpmlint tcc.spec ../RPMS/i386/tcc* ../SRPMS/tcc-0.9.24-1.fc9.src.rpm 
tcc.i386: W: spurious-executable-perm /usr/share/man/man1/tcc.1.gz
tcc-debuginfo.i386: E: empty-debuginfo-package
tcc-devel.i386: W: no-documentation
tcc-static.i386: W: no-documentation
5 packages and 1 specfiles checked; 1 errors, 3 warnings.

Do I need documentation in those packages?

Comment 1 Brennan Ashton 2008-11-26 08:18:26 UTC
oh and a koji scratch build here: http://koji.fedoraproject.org/koji/taskinfo?taskID=952263

Comment 2 Conrad Meyer 2008-11-26 08:32:01 UTC
Some problems:

Any reason the Summary isn't "Tiny C Compiler"?

You should convert Changelog to UTF-8 during prep, not build. Also, please use touch -r to keep the original timestamp.

The perl in %install can be trivially replaced with sed, part of the default build root (i.e. no extra BuildRequire). Also I'm not 100% sure that's doing the right thing there (probably you mean to replace just the install dir with DESTDIR+installdir, then pass DESTDIR to the make install?

In %files devel "%{_libdir}/%{name}/*.o" looks *very* wrong to me, the only things that should include straight object files are cross compilers IMO (maybe I'm wrong?).

Comment 3 Ralf Corsepius 2008-11-26 08:46:19 UTC
tcc had been the name of a Borland product (turbo c compiler) for a very long time (ca. a decade).

That said, I consider this package's name to be VERY misleading, if not illegal to use.

To circumvent such misunderstandings, I would advice upstream to rename their package. 

For now, blocking FEDORA-LEGAL.

Comment 4 Thomas Moschny 2008-11-26 08:57:52 UTC
Why not name the package TinyCC or tinycc? That are names the author also seems to use.

Comment 5 Ralf Corsepius 2008-11-26 09:36:17 UTC
(In reply to comment #4)
> Why not name the package TinyCC or tinycc?
Sounds reasonable to me.

What the actual executable called (Pardon, but I haven't looked into the actual package yet)? 

It had been "tcc" for TurboC. So if that also applies to TinyCC, this would be problematic, too.

IIRC (It's been many years since I used it) tcc also internally had used defines name __TCC__ etc. It would be technically very problematic, if TinyCC was doing the same, because such defines would likely clash with TurboC defines.

Comment 6 Brennan Ashton 2008-11-26 21:04:45 UTC
Ok I have changed the summary, you are right that is better.

On the topic of the name

tinytcc could be the name of the package, but what happens to the libtcc* and tcc*.h files?

That *.o pulls the bcheck.o part that give the "compile with built-in memory and bounds checker" option. Remove it and you will get:

tcc -b onemore.c 
tcc: file '/usr/lib/tcc/bcheck.o' not found

it is for debug only so that is why I put it in the devel package, is that wrong?

a grep check shows nothing like 
__tcc__ or __tinycc__ 
in any of the files

the executable is /usr/bin/tcc

Updated:
Spec URL: http://bashton.fedorapeople.org/tcc.spec
SRPM URL: http://bashton.fedorapeople.org/tcc-0.9.24-2.fc9.src.rpm

Comment 7 Susi Lehtola 2008-11-29 20:36:01 UTC
(In reply to comment #6)
> That *.o pulls the bcheck.o part that give the "compile with built-in memory
> and bounds checker" option. Remove it and you will get:
> 
> tcc -b onemore.c 
> tcc: file '/usr/lib/tcc/bcheck.o' not found
> 
> it is for debug only so that is why I put it in the devel package, is that
> wrong?

I think it should be in the main package, since you should be able to be able to compile with debug flags without having extra packages installed.

Comment 8 Tom "spot" Callaway 2008-12-01 16:24:04 UTC
The package naming is not illegal, it is not infringing on any relevant trademarks that I can find in the USTO. However, I can see how it would be misleading, and would suggest that the package be renamed to "tinycc". There should be no need to rename the libraries or executable.

Lifting FE-Legal.

Comment 9 Thomas Moschny 2009-02-11 14:42:43 UTC
Ping? Brennan, are you still interested in maintaining this package?

Comment 10 Brennan Ashton 2009-02-15 07:15:22 UTC
I am wrapping up some other projects early this week, and will get some of my packages reviews such as this one updated. I am removing the NEEDSPONSOR tag, as I have been sponsored for a while now. Thomas, are you wanting to co-maintain or something?

Comment 11 Thomas Moschny 2009-02-18 18:42:09 UTC
(In reply to comment #10)
> I am wrapping up some other projects early this week, and will get some of my
> packages reviews such as this one updated. I am removing the NEEDSPONSOR tag,
> as I have been sponsored for a while now. Thomas, are you wanting to
> co-maintain or something?

Not necessarily, but maybe. Well, I just think I'd occasionally use it, that's why I'd like to see it packaged, and I can help if needed ;)

Comment 12 Brennan Ashton 2009-02-19 03:58:53 UTC
Here is an updated spec and srpm:
http://bashton.fedorapeople.org/tinycc-0.9.24-3.fc10.src.rpm
http://bashton.fedorapeople.org/tinycc.spec

RPMLINT:

tinycc-devel.i386: W: no-documentation
tinycc-devel.i386: E: only-non-binary-in-usr-lib
tinycc-static.i386: W: no-documentation
5 packages and 1 specfiles checked; 1 errors, 2 warnings.

My understanding is that the Error is ok, as it is the same thing that gcc and other compilers do.

Comment 13 Jason Tibbitts 2009-03-09 04:56:23 UTC
The above links seem to be invalid.

Comment 14 Brennan Ashton 2009-03-09 05:26:16 UTC
The links were good, my account password expired and it killed them, it should be working in the next hour or two.

Comment 15 Jason Tibbitts 2009-03-13 01:38:40 UTC
Indeed, rpmlint produces those complaints; the only one that does concern me is the only-non-binary-in-usr-lib one.  There are a couple of questions:

What are the files in the -devel package used for?  If you need them to compile things with tinycc, I don't see the point in having a separate -devel package at all.  I tried running one of the example files without the -devel package installed and it failed due to a missing tcclib.h file, so it seems like you really do need the -devel package installed just to make use of the base package.

Are those files really arch-specific?  If they are (which is quite possible), then they're fine there.  If they aren't, then /usr/share is a better place.  Although I guess the point is academic since this only works on i386 anyway.

Comment 16 Brennan Ashton 2009-03-13 05:34:26 UTC
Here is an updated spec and srpm:
http://bashton.fedorapeople.org/tinycc-0.9.24-4.fc10.src.rpm
http://bashton.fedorapeople.org/tinycc.spec


The static is required to even think about running, so I merged that into the main package.  Devel is need for some programs but not all, I am not sure what to do, it is small and anything requiring stdio.h will require it.

I do not see why this would be bad here and ok with gcc?
tinycc-devel.i386: E: only-non-binary-in-usr-lib

The program has (from what I understand) and inherent SELinux issue.  To run correctly you need to disable the SELinux bool for memheap protection.  What should I do with this.  You can still compile and run, which defeats most of the purpose (still 10 time faster then gcc for may compiles).

As for being arch specific, I could see it go either way. I would be leaning towards yes, because of float.h which sets some variable sizes which may be different in 64bit.

Comment 17 Jason Tibbitts 2009-03-13 18:24:21 UTC
> I do not see why this would be bad here and ok with gcc?
> tinycc-devel.i386: E: only-non-binary-in-usr-lib

Well, rpm -ql gcc and look at what gcc puts into /usr/lib/gcc (even on x86_64, interestingly).  All sorts of object files and libraries, along with some headers.  Look at what this package puts into /usr/lib/tcc: Some header files.  Hence the question about whether they are arch-dependent and whether they would be better in /usr/share.  I don't think it's a huge issue, but the question was worth asking.  If you believe the files are arch specific (or would be if this worked on anything other than i386) then I think the files are OK where they are.

As for the selinux issue, please ask fedora-selinux-list for guidance.  You may be able to get some help there.

Comment 18 Ralf Corsepius 2009-03-14 04:16:04 UTC
(In reply to comment #17)
> > I do not see why this would be bad here and ok with gcc?
> > tinycc-devel.i386: E: only-non-binary-in-usr-lib
> 
> Well, rpm -ql gcc and look at what gcc puts into /usr/lib/gcc (even on x86_64,
> interestingly).  All sorts of object files and libraries, along with some
> headers.
These are GCC internal files and in fact are host-arch-independent from GCC's POV (compile-time demands). 

The reasons they are in /usr/lib/gcc instead of /usr/share are
widely historic (/usr/lib/<packagename> predates invention of /usr/share)
and related to run-time demands 
(/usr/lib/gcc/<target>/... may contain run-time-used shared libs).

>  Look at what this package puts into /usr/lib/tcc: Some header files.

IMO, this begs for more questions:
*  Is /usr/lib/libtcc.a located correctly?
Should it be a generally applicable library (e.g. usable by GCC compiled files) then this location is likely correct.

* Is /usr/include/libtcc.h located correctly?
I doubt it. IMO, it should be a tcc internal header, tcc should implicitly pull in interally from some internal include file search path.

Comment 19 Lubomir Rintel 2009-03-17 14:07:08 UTC
1.) devel subpackage

Why does this have a devel subpackage, and not just everything in one package? I'd say compilers are used exclusively for development.

2.) You don't install shared libraries

%post -p /sbin/ldconfig
%postun -p /sbin/ldconfig

This is not needed

3.) License

Most files are LGPLv2+, libtcc1.c is GPLv2+ with linking exception. Therefore the right license tag should probably be:

License: LGPLv2+ and GPLv2+ with exceptions

4.) libtcc

(In reply to comment #18)
[...]
> IMO, this begs for more questions:
> *  Is /usr/lib/libtcc.a located correctly?
> Should it be a generally applicable library (e.g. usable by GCC compiled files)
> then this location is likely correct.
> 
> * Is /usr/include/libtcc.h located correctly?
> I doubt it. IMO, it should be a tcc internal header, tcc should implicitly pull
> in interally from some internal include file search path.  

libtcc is not used internally. It's a compiler library that allows you to embed C compiler in your programs.

Therefore libtcc should be compiled dynamically, not statically. Also, guidelines for -devel packages would apply here.

Comment 20 Lubomir Rintel 2009-03-17 14:08:43 UTC
Please note that once you make libtcc a dynamic library, 2.) would no longer apply.

Comment 21 Lubomir Rintel 2009-03-17 14:10:15 UTC
5.) Compiler flags use

gcc -O2 -Wall -c -o libtcc1.o libtcc1.c
gcc -O2 -Wall -c -o bcheck.o bcheck.c

Why aren't optflags used here?

Comment 22 Lubomir Rintel 2009-03-22 18:12:28 UTC
Ping?

Comment 23 Lubomir Rintel 2009-03-30 19:30:42 UTC
Ping?

Comment 24 Lubomir Rintel 2009-04-14 19:00:44 UTC
Stale review. Closing.

Comment 25 Brennan Ashton 2009-04-15 05:09:51 UTC
Sorry, but I got a little short on time the last month or so.  It appears that this has some issues with SELINUX that are above me, and I am not interested in this package enough to track them down and submit patches to upstream.

Comment 26 Lubomir Rintel 2009-04-15 05:25:57 UTC
(In reply to comment #25)
> Sorry, but I got a little short on time the last month or so.  It appears that
> this has some issues with SELINUX that are above me, and I am not interested in
> this package enough to track them down and submit patches to upstream.  

If selinux's the only issue, I'll gladly help get the policy right; feel free to reopen and finish the package.

Comment 27 Brennan Ashton 2009-04-15 05:43:21 UTC
Ok I will take another stab at it this weekend.

Comment 28 Brennan Ashton 2009-04-20 03:19:04 UTC
Sorry ran out of time this weekend, I have a lot on my fedora plate right now.  I will try in the next couple days. It should not take too much time.

Comment 29 Lubomir Rintel 2009-05-08 07:12:53 UTC
ping

Comment 30 Henry Kroll 2010-04-21 22:50:05 UTC
* I pushed a Selinux patch upstream and made a git rpm based on the 2010 git: mob branch of tinycc and a fresh spec file with selinux patch. (I was unaware of this submission at the time.) If you would like to see it:
http://thenerdshow.com/rpm/tinycc-git20100114-0.fc13.src.rpm

Comment 31 Henry Kroll 2010-04-24 05:38:13 UTC
Now with cross-compilers!
http://thenerdshow.com/rpm/tinycc-git20100114-1.fc13.src.rpm

This build provides 2 packages, tinycc and tinycc-cross with cross-compilers for arm, c67 and win32. Builds on x86_64. Haven't tried other arch yet but it ought to work fine.

i386-win32-tcc fib.c
wine fib.exe 12
fib(12) = 144

The .spec file is at the 'gee it works stage.' There may be a few problems still.

Comment 33 Brennan Ashton 2010-04-28 05:47:36 UTC
I am done with school for the summer in a week I will take a look at these patches then.

Comment 34 Henry Kroll 2010-04-28 23:42:11 UTC
Thanks. I'm going fishing for the summer, so I won't be able to help much. :) 

Stealth update. Fixed minor typos. Added comments to Makefile. Pushed changes upstream: git push ssh://mob.cz/srv/git/tinycc.git mybranch:mob

The above SRPM generates the following:
tinycc-cross-git20100114-2.fc13.x86_64.rpm
tinycc-debuginfo-git20100114-2.fc13.x86_64.rpm
tinycc-git20100114-2.fc13.x86_64.rpm       (provides tinycc-devel, header files)
tinycc-libs-git20100114-2.fc13.x86_64.rpm  (shared lib, libtcc.so.1.0)

Comment 35 Henry Kroll 2010-10-27 11:26:12 UTC
1. Found a small bug with my selinux patch and fixed it.

2. Incorporated all the latest updates from the tinycc mob branch at
    http://repo.or.cz/w/tinycc.git/shortlog/refs/heads/mob

3. Rolled that into a new rpm, if somebody can review it
   http://thenerdshow.com/rpm/tinycc-git20101027-2.fc14.src.rpm

I've been trying it out on a variety of small test cases. It seems to be working really well now.

Comment 36 Susi Lehtola 2010-10-27 12:00:52 UTC
Cool, x86_64 support..? But the package doesn't compile on Fedora 13 x86_64.

Furthermore, your versioning is incorrect. According to the guidelines, your version should be something like

Version: 0.9.26
Release: 0.1.20100127git

since this is a prerelease version. Or, if you consider it post-release, use 0.9.25 as version and e.g. 1.20100127git as release.

And you might want to use a newer snapshot, see http://repo.or.cz/w/tinycc.git/shortlog/refs/heads/mob

Comment 37 Henry Kroll 2010-10-29 20:02:05 UTC
Mock won't build cross compilers on x86_64 without glibc-devel.i686
Blocked by https://bugzilla.redhat.com/show_bug.cgi?id=440368

Manually install glibc-devel.i686 and it will build win32 cross on x86_64.

For some reason it wasn't pulling in the latest mob changes.
This is built from
http://repo.or.cz/w/tinycc.git/shortlog/refs/heads/mob

http://thenerdshow.com/rpm/tinycc-0.9.26-0.1.20101029git.fc14.src.rpm

Had to work around some bugs to get it to build on mock. At the moment cross-compilers will not build on mock due to bug 440368. You can build the cross-compiler rpm easily as a test user by installing glibc-devel.i686. For some reason mock says it is not available in the repos.

1) %if %{?buildcross}
# FIXME: cross-compilers on x86_64 require glibc-devel.i686
# Blocked by https://bugzilla.redhat.com/show_bug.cgi?id=440368
# BuildRequires glibc-devel.i686 doesn't pull in dependency. Why?

2) makeinfo permission denied bug
# FIXME: mock
# make: execvp: makeinfo: Permission denied
Patch0: tcc-makeinfo-bug-workaround.patch

Comment 38 Marc-Andre Lureau 2011-06-01 11:05:42 UTC
(fwiw, the latest srpm seems to be now here:
http://thenerdshow.com/rpm/tinycc-0.9.26-0.1.20101220git.fc14.src.rpm)


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