Bug 223591 - Review Request: Magic - A very capable VLSI layout tool
Review Request: Magic - A very capable VLSI layout tool
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2007-01-20 07:29 EST by Chitlesh GOORAH
Modified: 2008-12-28 14:24 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-02-26 15:08:06 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
chitlesh: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
Mock build log of magic-7.4.33-4.fc7 (223.79 KB, text/plain)
2007-02-04 09:05 EST, Mamoru TASAKA
no flags Details
Mock build log of magic-7.4.33-4.fc7 (again) (223.42 KB, text/plain)
2007-02-05 10:40 EST, Mamoru TASAKA
no flags Details

  None (edit)
Description Chitlesh GOORAH 2007-01-20 07:29:54 EST
Spec URL: http://tux.u-strasbg.fr/~chit/RPMS/magic.spec
SRPM URL: http://tux.u-strasbg.fr/~chit/RPMS/magic-7.4.33-1.src.rpm
Description:
Magic is a venerable VLSI layout tool, written in the 1980's at
Berkeley by John Ousterhout, now famous primarily for writing
the scripting interpreter language Tcl. Due largely in part to
its liberal Berkeley open-source license, magic has remained
popular with universities and small companies.

Magic is widely cited as being the easiest tool to use for
circuit layout, even for people who ultimately rely on commercial
tools for their product design flow.
Comment 1 Chitlesh GOORAH 2007-01-20 07:49:47 EST
The following rpmlints can be ignored:
E: magic script-without-shebang /usr/lib/magic/tcl/console.tcl
=> Useful for launching magic /usr/lib/magic/tcl/magicexec

W: magic hidden-file-or-dir /usr/lib/magic/sys/.magicrc
=> On launching magic, you can see on the tkcon main:
[..]
MOSIS Scalable CMOS Technology for Standard Rules
Processing system .magicrc file
New windows will not have a title caption.
[..]

E: magic-doc only-non-binary-in-usr-lib
W: magic-doc no-documentation
Documentation are at:
/usr/lib/magic/doc

The copyright file is /usr/lib/magic/doc/copyright.ps

Comment 2 Parag AN(पराग) 2007-01-23 04:32:59 EST
Can't see LICENSE text in %doc package.
rpmlint is OK then.
mock build is fine.
But can't see Desktop file so used this application from console
Maybe you like to move Categories from Science to Engineering in .desktop file.
Comment 4 Chitlesh GOORAH 2007-01-31 23:58:15 EST
Is there some chance that this package be reviewed before this weekend ?
Comment 5 Parag AN(पराग) 2007-02-01 00:00:25 EST
Sorry for being late. Got a lot of work. Will do review by end of day today.
Comment 6 Parag AN(पराग) 2007-02-01 00:44:11 EST
Review:
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM.
- rpmlint is NOT silent for RPMS.
+ source match upstream.
f35f93bdb9ae3842879d52e08c6d7ace  magic-7.4.33.tgz
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.  License text included in package.
+ %doc is large so added -doc subpackage.
+ %doc does not affect runtime.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code Not contents.
+ no static libraries present.
+ no .pc files present.
+ no -devel subpackage exists.
+ no .la files.
+ no translations are available
+ Dose owns the directories it creates.
+ no duplicates in %files.
+ icon cache scriptlets used.
+ Desktop file handled correctly.
+ file permissions are appropriate.
+ GUI app
APPROVED.

Comment 7 Chitlesh GOORAH 2007-02-01 03:27:16 EST
Updated:
Spec URL: http://tux.u-strasbg.fr/~chit/RPMS/magic.spec
SRPM URL: http://tux.u-strasbg.fr/~chit/RPMS/magic-7.4.33-3.src.rpm

Fix for CFLAGS

thanks for the review
Comment 8 Mamoru TASAKA 2007-02-01 11:20:55 EST
Well,

* Same as irsim (bug 226715), %clean stage does not
  remove all files expanded from the original source correctly.
* CFLAGS can be dealt with the same way as I pointed out
  in irsim.

* I cannot understand why documentations should be included
  under %{_libdir}. 
  Are these files used at runtime by
  binaries/scripts included in magic related rpm?
  - If so, I think that the documentation should not be split out.
  - If not, I think that all documentaion files should be moved
    under %{_datadir}/doc.
* By the way, why does this package have to update gtk icon
  cache?
Comment 9 Chitlesh GOORAH 2007-02-03 16:08:54 EST
Updated:
Spec URL: http://tux.u-strasbg.fr/~chit/RPMS/magic.spec
SRPM URL: http://tux.u-strasbg.fr/~chit/RPMS/magic-7.4.33-4.src.rpm

See whether it's ok for you, before I'll commit to cvs
Comment 10 Mamoru TASAKA 2007-02-04 09:05:01 EST
Created attachment 147302 [details]
Mock build log of  magic-7.4.33-4.fc7

Mock build log of  magic-7.4.33-4 on FC-devel i386.

* Currently I cannot figure out why mockbuild log says:
-----------------------------------------------
BLT:	      no

  Tcl/Tk magic uses the BLT package to create a tree diagram
  of the cell hierarchy in a design.  Without it, this option
  is unavailable.  Consider installing the BLT package.
-----------------------------------------------

* icon cache updating
-----------------------------------------------
%post
touch --no-create %{_datadir}/icons/hicolor || :
%{_bindir}/gtk-update-icon-cache --quiet %{_datadir}/icons/hicolor || :

%postun
touch --no-create %{_datadir}/icons/hicolor || :
%{_bindir}/gtk-update-icon-cache --quiet %{_datadir}/icons/hicolor || :
--------------------------------------------------
  - I don't think this is needed because this package
    installs no icon image files into the directory.

* rpmlint
  says..
--------------------------------------------------
W: magic hidden-file-or-dir /usr/lib/magic/sys/.magicrc
E: magic script-without-shebang /usr/lib/magic/tcl/console.tcl
--------------------------------------------------
  * What is the formar file?
  * I think the latter is surely the issue.

* Please consider to make the log of compile more
  precise, not just telling us:
--------------------------------------------------
--- compiling cmwind/CMWcmmnds.o
--- compiling cmwind/CMWundo.o
--- compiling cmwind/CMWrgbhsv.o
--- linking libcmwind.o
--------------------------------------------------
   I cannot find what is actually done here.

* Documentation
  - main package contains the files under
    /usr/lib/magic/doc, which seem to be the same files
    in -doc file.
Comment 11 Parag AN(पराग) 2007-02-05 06:32:58 EST
Chitlesh,
 I think it will be good to answer above comment first before i will proceed on
RE-approving this package.
Comment 12 Chitlesh GOORAH 2007-02-05 06:42:20 EST
(In reply to comment #10)
> Created an attachment (id=147302) [edit]
> Mock build log of  magic-7.4.33-4.fc7
> 
> Mock build log of  magic-7.4.33-4 on FC-devel i386.
> 
> * Currently I cannot figure out why mockbuild log says:
> -----------------------------------------------
> BLT:	      no
> 
>   Tcl/Tk magic uses the BLT package to create a tree diagram
>   of the cell hierarchy in a design.  Without it, this option
>   is unavailable.  Consider installing the BLT package.
> -----------------------------------------------

I can't see why in rawhide it refused to detect it. However the blt is been
installed.
Installed: blt.i386 0:2.4-14.z.fc6

Comment 13 Parag AN(पराग) 2007-02-05 07:02:42 EST
(In reply to comment #10)
> Created an attachment (id=147302) [edit]
> Mock build log of  magic-7.4.33-4.fc7
> 
> Mock build log of  magic-7.4.33-4 on FC-devel i386.
> 
> * Currently I cannot figure out why mockbuild log says:
> -----------------------------------------------
> BLT:	      no
> 
>   Tcl/Tk magic uses the BLT package to create a tree diagram
>   of the cell hierarchy in a design.  Without it, this option
>   is unavailable.  Consider installing the BLT package.
> -----------------------------------------------
> 
 mock build showed me this as Yes

> * icon cache updating
> -----------------------------------------------
> %post
> touch --no-create %{_datadir}/icons/hicolor || :
> %{_bindir}/gtk-update-icon-cache --quiet %{_datadir}/icons/hicolor || :
> 
> %postun
> touch --no-create %{_datadir}/icons/hicolor || :
> %{_bindir}/gtk-update-icon-cache --quiet %{_datadir}/icons/hicolor || :
> --------------------------------------------------
>   - I don't think this is needed because this package
>     installs no icon image files into the directory.
 as per given on http://fedoraproject.org/wiki/Packaging/ScriptletSnippets, I
could not find any files being installed in %{_datadir}/icons. So you can remove
this scriptlets.
> 
> * rpmlint
>   says..
> --------------------------------------------------
> W: magic hidden-file-or-dir /usr/lib/magic/sys/.magicrc
> E: magic script-without-shebang /usr/lib/magic/tcl/console.tcl
> --------------------------------------------------
>   * What is the formar file?
>   * I think the latter is surely the issue.

  I think you have given some explanation for that in comment #1
> 
> * Please consider to make the log of compile more
>   precise, not just telling us:
> --------------------------------------------------
> --- compiling cmwind/CMWcmmnds.o
> --- compiling cmwind/CMWundo.o
> --- compiling cmwind/CMWrgbhsv.o
> --- linking libcmwind.o
> --------------------------------------------------
>    I cannot find what is actually done here.
   Looks like make command is not providing that info.
> 
> * Documentation
>   - main package contains the files under
>     /usr/lib/magic/doc, which seem to be the same files
>     in -doc file.

 yes. i found them similar from -doc to main package. any reason for adding doc
files again in main package also?
Comment 14 Chitlesh GOORAH 2007-02-05 07:09:49 EST
(In reply to comment #13)
> (In reply to comment #10)
> > Created an attachment (id=147302) [edit] [edit]
> > Mock build log of  magic-7.4.33-4.fc7
> > 
> > Mock build log of  magic-7.4.33-4 on FC-devel i386.
> > 
> > * Currently I cannot figure out why mockbuild log says:
> > -----------------------------------------------
> > BLT:	      no
> > 
> >   Tcl/Tk magic uses the BLT package to create a tree diagram
> >   of the cell hierarchy in a design.  Without it, this option
> >   is unavailable.  Consider installing the BLT package.
> > -----------------------------------------------
> > 
>  mock build showed me this as Yes

Well I saw it as Yes too before submitting. I fear that something have changed
in the development repos.

If this isn't fixed, I'll wait.

And consider the below issues fixed.
Comment 15 Parag AN(पराग) 2007-02-05 07:13:47 EST
(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #10)
> > > Created an attachment (id=147302) [edit] [edit] [edit]
> > > Mock build log of  magic-7.4.33-4.fc7
> > > 
> > > Mock build log of  magic-7.4.33-4 on FC-devel i386.
> > > 
> > > * Currently I cannot figure out why mockbuild log says:
> > > -----------------------------------------------
> > > BLT:	      no
> > > 
> > >   Tcl/Tk magic uses the BLT package to create a tree diagram
> > >   of the cell hierarchy in a design.  Without it, this option
> > >   is unavailable.  Consider installing the BLT package.
> > > -----------------------------------------------
> > > 
> >  mock build showed me this as Yes
> 
> Well I saw it as Yes too before submitting. I fear that something have changed
> in the development repos.
> 
> If this isn't fixed, I'll wait.
> 
> And consider the below issues fixed.
 Sorry, Which one?
Comment 16 Mamoru TASAKA 2007-02-05 10:40:57 EST
Created attachment 147361 [details]
Mock build log of  magic-7.4.33-4.fc7 (again)

(In reply to comment #10)
> * Currently I cannot figure out why mockbuild log says:
> -----------------------------------------------
> BLT:	      no

Well, rawhide released update for tcl and this time
checking BLT turned to YES. Perhaps Jakub Jelinek
fixed the problem.

So please modify the other issues.
Comment 18 Mamoru TASAKA 2007-02-06 12:00:47 EST
Well, I have to say again that tcl 8.5 still complains
about "BLT: no"...
however, as long as I checked the souce code,
this does not change the result of compilation, so
I think this can be ignored.

For me -5 seems okay, just one question.
- Why does some tcl files have executable permission with shebang,
  while some don't? Should all files should have both (i.e.
  executable permission with shebang) or all files should not,
  or current state has some meaning? (I think this may not a
  big issue, however, I just want to know what is occurring here).

= A note:
  copyright.ps disappeared. While copyright.ps seems to say that
  this is MIT, however, as long as I read bug 226715, upstream want
  to claim that this is GPL. So currently copyright.ps can be ignored
  (my recognition is that the upstream of irsim and magic is the
   same, is this correct?).
Comment 19 Chitlesh GOORAH 2007-02-09 11:29:36 EST
(In reply to comment #18)
> For me -5 seems okay, just one question.
> - Why does some tcl files have executable permission with shebang,
>   while some don't? Should all files should have both (i.e.
>   executable permission with shebang) or all files should not,
>   or current state has some meaning? (I think this may not a
>   big issue, however, I just want to know what is occurring here).

I did some tests and concluded that I can chmod -x those rpmlint shebangs. Parag
do you want me to dump another release ?

> = A note:
>   copyright.ps disappeared. While copyright.ps seems to say that
>   this is MIT, however, as long as I read bug 226715, upstream want
>   to claim that this is GPL. So currently copyright.ps can be ignored
>   (my recognition is that the upstream of irsim and magic is the
>    same, is this correct?).

Yes it's the same upstream : http://opencircuitdesign.com/ 
Magic is the last one on the list to pave in fedora repositories :)

The below apps are mantained by  http://opencircuitdesign.com/ 
    * Magic, the VLSI layout editor, extraction, and DRC tool.
    * XCircuit, the circuit drawing and schematic capture tool.
    * IRSIM, the switch-level digital circuit simulator.
    * Netgen, the circuit netlist comparison (LVS) and netlist conversion tool.
    * PCB, the printed circuit board layout editor.
Comment 20 Parag AN(पराग) 2007-02-11 23:32:08 EST
Yes that will be good to bump to next release number.
Comment 22 Parag AN(पराग) 2007-02-22 00:17:42 EST
checked again in mock build. Looks ok to me.
rpmlint however reports single warning which already got explanation from reporter.
W: magic hidden-file-or-dir /usr/lib/magic/sys/.magicrc

APPROVED.
Comment 23 Chitlesh GOORAH 2007-02-22 03:16:12 EST
New Package CVS Request
=======================
Package Name: magic
Short Description: A very capable VLSI layout tool
Owners: cgoorah@yyahoo.com.au
Branches: FC-6
InitialCC: mtasaka@ioa.s.u-tokyo.ac.jp
Comment 24 Mamoru TASAKA 2007-02-26 13:36:15 EST
Chitlesh, perhaps your mail address has a typo?
It seems that the owner of magic is registered as
yyahoo, not yahoo.
Comment 25 Chitlesh GOORAH 2007-02-26 13:44:26 EST
(In reply to comment #24)
> Chitlesh, perhaps your mail address has a typo?
> It seems that the owner of magic is registered as
> yyahoo, not yahoo.

Yes, true. I'll contact warren to see whether he can change it or not on owners.list
Comment 26 Chitlesh GOORAH 2008-12-27 21:54:25 EST
Package Change Request
=======================
Package Name: magic
Short Description: A very capable VLSI layout tool
Owners: chitlesh
Branches: EL-5
Comment 27 Kevin Fenzi 2008-12-28 14:24:11 EST
cvs done.

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