Bug 473037 - (tinycc) Review Request: tinycc - Tiny C Compiler
Review Request: tinycc - Tiny C Compiler
Status: CLOSED DEFERRED
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Lubomir Rintel
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-11-26 03:15 EST by Brennan Ashton
Modified: 2011-06-01 07:05 EDT (History)
13 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-05-08 03:12:53 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lkundrak: fedora‑review?


Attachments (Terms of Use)

  None (edit)
Description Brennan Ashton 2008-11-26 03:15:59 EST
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 03:18:26 EST
oh and a koji scratch build here: http://koji.fedoraproject.org/koji/taskinfo?taskID=952263
Comment 2 Conrad Meyer 2008-11-26 03:32:01 EST
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 03:46:19 EST
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 03:57:52 EST
Why not name the package TinyCC or tinycc? That are names the author also seems to use.
Comment 5 Ralf Corsepius 2008-11-26 04:36:17 EST
(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 16:04:45 EST
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 15:36:01 EST
(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 11:24:04 EST
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 09:42:43 EST
Ping? Brennan, are you still interested in maintaining this package?
Comment 10 Brennan Ashton 2009-02-15 02:15:22 EST
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 13:42:09 EST
(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-18 22:58:53 EST
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 00:56:23 EDT
The above links seem to be invalid.
Comment 14 Brennan Ashton 2009-03-09 01:26:16 EDT
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-12 21:38:40 EDT
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 01:34:26 EDT
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 14:24:21 EDT
> 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 00:16:04 EDT
(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 10:07:08 EDT
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 10:08:43 EDT
Please note that once you make libtcc a dynamic library, 2.) would no longer apply.
Comment 21 Lubomir Rintel 2009-03-17 10:10:15 EDT
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 14:12:28 EDT
Ping?
Comment 23 Lubomir Rintel 2009-03-30 15:30:42 EDT
Ping?
Comment 24 Lubomir Rintel 2009-04-14 15:00:44 EDT
Stale review. Closing.
Comment 25 Brennan Ashton 2009-04-15 01:09:51 EDT
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 01:25:57 EDT
(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 01:43:21 EDT
Ok I will take another stab at it this weekend.
Comment 28 Brennan Ashton 2009-04-19 23:19:04 EDT
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 03:12:53 EDT
ping
Comment 30 Henry Kroll 2010-04-21 18:50:05 EDT
* 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 01:38:13 EDT
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 01:47:36 EDT
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 19:42:11 EDT
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@repo.or.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 07:26:12 EDT
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 08:00:52 EDT
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 16:02:05 EDT
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 07:05:42 EDT
(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.