Bug 223591
Summary: | Review Request: Magic - A very capable VLSI layout tool | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Chitlesh GOORAH <chitlesh> | ||||||
Component: | Package Review | Assignee: | Parag AN(पराग) <panemade> | ||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||||
Severity: | medium | Docs Contact: | |||||||
Priority: | medium | ||||||||
Version: | rawhide | CC: | mtasaka | ||||||
Target Milestone: | --- | Flags: | chitlesh:
fedora-review+
kevin: fedora-cvs+ |
||||||
Target Release: | --- | ||||||||
Hardware: | All | ||||||||
OS: | Linux | ||||||||
Whiteboard: | |||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||
Doc Text: | Story Points: | --- | |||||||
Clone Of: | Environment: | ||||||||
Last Closed: | 2007-02-26 20:08:06 UTC | Type: | --- | ||||||
Regression: | --- | Mount Type: | --- | ||||||
Documentation: | --- | CRM: | |||||||
Verified Versions: | Category: | --- | |||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||
Embargoed: | |||||||||
Bug Depends On: | |||||||||
Bug Blocks: | 163779 | ||||||||
Attachments: |
|
Description
Chitlesh GOORAH
2007-01-20 12:29:54 UTC
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 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. 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-2.src.rpm Is there some chance that this package be reviewed before this weekend ? Sorry for being late. Got a lot of work. Will do review by end of day today. 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. 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 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? 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 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.
Chitlesh, I think it will be good to answer above comment first before i will proceed on RE-approving this package. (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 (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? (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. (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? 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. 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-5.src.rpm 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?). (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. Yes that will be good to bump to next release number. 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-6.src.rpm 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. New Package CVS Request ======================= Package Name: magic Short Description: A very capable VLSI layout tool Owners: cgoorah.au Branches: FC-6 InitialCC: mtasaka.u-tokyo.ac.jp Chitlesh, perhaps your mail address has a typo? It seems that the owner of magic is registered as yyahoo, not yahoo. (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 Package Change Request ======================= Package Name: magic Short Description: A very capable VLSI layout tool Owners: chitlesh Branches: EL-5 cvs done. |