Bug 206693 - Review Request: KTechlab - Development and simulation of microcontrollers and electronic circuits
Review Request: KTechlab - Development and simulation of microcontrollers and...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
: Reopened
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-09-15 14:19 EDT by Chitlesh GOORAH
Modified: 2008-11-05 18:49 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-11-05 10:49:12 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
chitlesh: fedora‑review+
a.badger: fedora‑cvs+


Attachments (Terms of Use)
build.log for ppc (198.89 KB, text/plain)
2006-10-13 08:28 EDT, Chitlesh GOORAH
no flags Details

  None (edit)
Description Chitlesh GOORAH 2006-09-15 14:19:48 EDT
Spec URL: http://chitlesh.googlepages.com/ktechlab.spec
SRPM URL: http://chitlesh.googlepages.com/ktechlab-0.3-1.src.rpm
Description:
KTechlab is a development and simulation environment for microcontrollers
and electronic circuits, distributed under the GNU General Public License.

KTechlab consists of several well-integrated components:
A circuit simulator, capable of simulating logic, linear devices and some
nonlinear devices.
* Integration with gpsim, allowing PICs to be simulated in circuit.
* A schematic editor, which provides a rich real-time feedback of the
simulation.
* A flowchart editor, allowing PIC programs to be constructed visually.
* MicroBASIC; a BASIC-like compiler for PICs, written as a companion program
to KTechlab.
* An embedded Kate part, which provides a powerful editor for PIC programs.
* Integrated assembler and disassembler via gpasm and gpdasm.
Comment 1 Chitlesh GOORAH 2006-09-15 14:25:11 EDT
Some examples at http://ktechlab.org/screenshots/ for testing.
Comment 2 Ralf Corsepius 2006-09-16 01:41:02 EDT
Some remarks:

1. I don't understand, why you
- BR: autoconf
It's not required. Building the package invokes autoheader due to a bug
somewhere in the source tarball, nevertheless this invocation of autoheader does
nothing.

- BR: libtool
No idea why you do this.

- R: gpsim
The package doesn't directly depend on gpsim. It depends on the libs from gpsim,
which are automatically being pulled in by rpm.

- R: gputils
I don't see any dependency on this package.


2. The tarball is mal-packaged.
It ships an autom4te.cache/.

I'd suggest to rm -rf autom4te.cache in %prep.
Comment 3 Mamoru TASAKA 2006-09-16 02:51:38 EDT
Ralf, thank you for adding some remarks:

Well, I will review this package.

1. From http://fedoraproject.org/wiki/Packaging/Guidelines :

* Requires
  - Explain why this package requires gputils, gpsim.

* BuildRequires
  - libtool, autoconf is not necessary, perhaps.

* Scriptlets requirements
  - According to http://fedoraproject.org/wiki/Packaging/ScriptletSnippets ,
    Requires(post):   desktop-file-utils
    Requires(postun): desktop-file-utils
    should be removed.

* File and Directory Ownership
  - The following directries are not owned.
    /usr/share/config.kcfg/

2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
  = Nothing.

3. Other things I have noticed:
   ? Well, I would appreciate it when someone would check if
     this package can be build on x86_64 as this uses qt-devel.
Comment 4 Mamoru TASAKA 2006-09-16 02:54:23 EDT
Also, check if gettext is requires as BuildRequires.
Comment 5 Mamoru TASAKA 2006-09-16 03:58:10 EDT
Ah, also ldconfig on %post, %postun is not required.
Comment 6 Mamoru TASAKA 2006-09-20 11:25:06 EDT
ping?
Comment 7 Chitlesh GOORAH 2006-09-20 18:55:57 EDT
(In reply to comment #3)
> Well, I will review this package.
> 
Thanks, Mamoru

> 1. From http://fedoraproject.org/wiki/Packaging/Guidelines :
> 
> * Requires
>   - Explain why this package requires gputils, gpsim.
> 
gpsim, for pic simulation:
PIC microcontrollors are fully simulated in the circuit, via integration with
gpsim. All PICs supported by gpsim are capable of being run and debugged in
KTechlab, but only bidirectional and open PIC pins are simulated properly.

gputils for assembly:
KTechlab provides integration with gputils for quick assembly and testing of
progams. Errors in the assembly output can be clicked on to go straight to the
problem code.
> * BuildRequires
>   - libtool, autoconf is not necessary, perhaps.
> 
 
> * File and Directory Ownership
>   - The following directries are not owned.
>     /usr/share/config.kcfg/
> 

This is owned by kdebase, hence I should add it as Requires
ls /usr/share/config.kcfg/
Comment 8 Chitlesh GOORAH 2006-09-20 19:05:41 EDT
(In reply to comment #2)

> 2. The tarball is mal-packaged.
> It ships an autom4te.cache/.
> 
> I'd suggest to rm -rf autom4te.cache in %prep.

I can't see it.
rpm -ql ktechlab | grep cache
/usr/share/doc/HTML/en/ktechlab/index.cache.bz2
Comment 9 Ralf Corsepius 2006-09-20 20:40:23 EDT
(In reply to comment #8)
> (In reply to comment #2)
> 
> > 2. The tarball is mal-packaged.
> > It ships an autom4te.cache/.
# tar tjvf ktechlab-0.3.tar.bz2 | grep autom4
drwxr-xr-x david/david       0 2006-01-05 11:55 ktechlab-0.3/autom4te.cache/
-rw-r--r-- david/david    5185 2006-01-05 11:55 ktechlab-0.3/autom4te.cache/requests
-rw-r--r-- david/david 1035902 2006-01-05 11:55 ktechlab-0.3/autom4te.cache/output.0
-rw-r--r-- david/david  200071 2006-01-05 11:55 ktechlab-0.3/autom4te.cache/traces.0
Comment 10 Ralf Corsepius 2006-09-20 20:44:07 EDT
(In reply to comment #7)
> (In reply to comment #3)

> >   - Explain why this package requires gputils, gpsim.
> > 
> gpsim, for pic simulation:

> gputils for assembly:
Show us any spot inside of the code which invokes any tool from these packages.

ktechlab is linked against libraries from these packages, but I don't see it
invoking any executable from these packages.
Comment 11 Mamoru TASAKA 2006-09-21 08:41:38 EDT
Well,

(In reply to comment #7)
> This is owned by kdebase, hence I should add it as Requires

Is kdebase required only for the dependency for /usr/share/config.kcfg/ ?
Actually I don't use KDE and currently I don't have kdelibs installed.
However, this package seems to work well (at least by checking:
   ktechlab examples.circuit
and examples.circuit is from http://ktechlab.org/screenshots/ ).

So, if it is only for the ownership problem that kdelibs should be required,
I think that /usr/share/config.kcfg/ should be owned by ktechlab, too and
kdebase should not be required.

And..
> gputils for assembly:
> KTechlab provides integration with gputils for quick assembly and testing of
> progams.

Well, this means that it is _useful_ that KTechlab is installed together with
gputils, however, this does not mean that KTechlab is completely unusable 
without gputils? 
For me, executing "ktechlab examples.circuit" seems to work without gputils .
Well, I cannot judge correctly if this is working as you desired, however,
at least ktechlab seems to launch normally and read circult correctly.

IMO, the explicit dependency for gputils, gpsim can be removed.

Also, check for BuildRequires and something else.
Comment 13 Mamoru TASAKA 2006-09-23 08:33:50 EDT
Okay!!!

This package (ktechlab) is APPROVED by me.

P.S. again I would appreciate it if you would review my request (bug 206487 ).
Comment 14 Alain Portal 2006-09-23 10:13:43 EDT
I'm not agree with the removal of Requires gputils gpsim.

Perhaps Requires gpsim isn't needed because rpm complain about dependencies:
[root@lns-bzn-37-82-253-50-64 i386]# LANG=C rpm -ivh ktechlab-0.3-2.i386.rpm
error: Failed dependencies:
        libgpsim.so.0 is needed by ktechlab-0.3-2.i386
        libgpsim_eXdbm.so.0 is needed by ktechlab-0.3-2.i386
        libgpsimcli.so.0 is needed by ktechlab-0.3-2.i386

Of course, "ktechlab examples.circuit" work without gputils, because the 
circuit is an analog circuit, there is no PIC in it.

But take the second example, "code_entry.circuit".

Ktechlab open an error dialog box: "Assembly failed. Please check you have 
gputils installed"

Ktechlab requires gputils for PIC simulation.


Comment 15 Mamoru TASAKA 2006-09-23 10:31:22 EDT
(In reply to comment #14)
> I'm not agree with the removal of Requires gputils gpsim.
> 
> Ktechlab requires gputils for PIC simulation.
> 

Well, if it is common that Ktechlab is used for PIC simulation , in which case
gputils is required generally, making Ktechlab require gputils is reasonable.

I leave the judgment of whether Ktechlab should require gputils to Chitlesh.
Anyway this package is approved.
Comment 16 Alain Portal 2006-09-23 13:39:32 EDT
Chitlesh,

I don't know if you saw that: http://ktechlab.org/download/gpsim.php

I commited the patch and I'll ask for rebuild on monday.
Comment 17 Ralf Corsepius 2006-09-24 00:49:59 EDT
(In reply to comment #15)
> (In reply to comment #14)
> > I'm not agree with the removal of Requires gputils gpsim.
> > 
> > Ktechlab requires gputils for PIC simulation.
> > 
> 
> Well, if it is common that Ktechlab is used for PIC simulation , in which case
> gputils is required generally, making Ktechlab require gputils is reasonable.

Quite easy to prove/counter-prove: Show us the piece of source code that
introduces this dependency on gputils!
Comment 18 Mamoru TASAKA 2006-09-24 01:00:20 EDT
(In reply to comment #17)
> (In reply to comment #15)
> > (In reply to comment #14)
> > > I'm not agree with the removal of Requires gputils gpsim.
> > > 
> > > Ktechlab requires gputils for PIC simulation.
> > > 
> > 
> > Well, if it is common that Ktechlab is used for PIC simulation , in which case
> > gputils is required generally, making Ktechlab require gputils is reasonable.
> 
> Quite easy to prove/counter-prove: Show us the piece of source code that
> introduces this dependency on gputils!

Well, autually:
[tasaka1@localhost ktechlab-0.3]$ grep -r 'gputils' .
./doc/en/flowcode.docbook:              <para>Of course, if you don't have
gputils installed - with which <command>gpasm</command> is distributed - then
the last step can't be performed.</para>
./src/languages/gpdasm.cpp:             KMessageBox::sorry(
LanguageManager::self()->logView(), i18n("Disassembly failed. Please check you
have gputils installed.") );
./src/languages/gpasm.cpp:              KMessageBox::sorry(
LanguageManager::self()->logView(), i18n("Assembly failed. Please check you have
gputils installed.") );
./src/languages/gplink.cpp:             KMessageBox::sorry(
LanguageManager::self()->logView(), i18n("Linking failed. Please check you have
gputils installed.") );
./src/languages/gplib.cpp:              KMessageBox::sorry(
LanguageManager::self()->logView(), i18n("Linking failed. Please check you have
gputils installed.") );


Comment 19 Mamoru TASAKA 2006-09-24 01:10:33 EDT
And...

[tasaka1@localhost ktechlab-0.3-2]$ for f in `rpm -qlp
gputils-0.13.4-1.fc6.i386.rpm | sed -n -e 's|/usr/bin/||p'` ; do grep -r
"[^A-Za-z]$f[^A-Za-z\.]" . ; done | grep cpp:
./ktechlab-0.3/microbe/microbe.cpp:     // Things starting with b are reserved
by gpasm (for binary numbers)
./ktechlab-0.3/src/gui/settingsdlg.cpp: addPage( m_gpasmSettingsWidget, "Gpasm",
"convert_to_hex", "gpasm" );
./ktechlab-0.3/src/languages/processchain.cpp:          INDIRECT_PROCESS(      
AssemblyAbsolute_PIC,                   gpasm,        ".hex" )
./ktechlab-0.3/src/languages/processchain.cpp:          DIRECT_PROCESS(        
AssemblyAbsolute_Program,               gpasm )
./ktechlab-0.3/src/languages/processchain.cpp:          INDIRECT_PROCESS(      
AssemblyRelocatable_Library,    gpasm,          ".o" )
./ktechlab-0.3/src/languages/processchain.cpp:          DIRECT_PROCESS(        
AssemblyRelocatable_Object,             gpasm )
./ktechlab-0.3/src/languages/processchain.cpp:          INDIRECT_PROCESS(      
AssemblyRelocatable_PIC,                gpasm,        ".o" )
./ktechlab-0.3/src/languages/processchain.cpp:          INDIRECT_PROCESS(      
AssemblyRelocatable_Program,    gpasm,          ".o" )
./ktechlab-0.3/src/languages/processchain.cpp:LanguageFunction( Gpasm, gpasm,
m_pGpasm )
./ktechlab-0.3/src/languages/gpasm.cpp: *m_languageProcess << ("gpasm");
./ktechlab-0.3/src/languages/gpdasm.cpp:        *m_languageProcess << ("gpdasm");
./ktechlab-0.3/src/languages/processchain.cpp:          DIRECT_PROCESS(        
Object_Disassembly,                             gpdasm )
./ktechlab-0.3/src/languages/processchain.cpp:          DIRECT_PROCESS(        
Program_Disassembly,                    gpdasm )
./ktechlab-0.3/src/languages/processchain.cpp:LanguageFunction( Gpdasm, gpdasm,
m_pGpdasm )
./ktechlab-0.3/src/languages/processchain.cpp:          DIRECT_PROCESS(        
Object_Library,                                 gplib )
./ktechlab-0.3/src/languages/processchain.cpp:LanguageFunction( Gplib, gplib,
m_pGplib )
./ktechlab-0.3/src/languages/gplib.cpp: *m_languageProcess << ("gplib");
./ktechlab-0.3/src/gui/projectdlgs.cpp: //BEGIN Update gplink options
./ktechlab-0.3/src/gui/projectdlgs.cpp: //END Update gplink options
./ktechlab-0.3/src/languages/processchain.cpp:          INDIRECT_PROCESS(      
Object_PIC,                                           gplink,          ".lib" )
./ktechlab-0.3/src/languages/processchain.cpp:          DIRECT_PROCESS(        
Object_Program,                                 gplink )
./ktechlab-0.3/src/languages/processchain.cpp:LanguageFunction( Gplink, gplink,
m_pGplink )
./ktechlab-0.3/src/languages/gplink.cpp:        *m_languageProcess << ("gplink");
Comment 20 Ralf Corsepius 2006-09-24 02:18:07 EDT
(In reply to comment #18)
No call to gputils ;)


(In reply to comment #19)
OK, these probably contain calls to gputils.
Comment 21 Alain Portal 2006-09-24 08:41:40 EDT
gputils is the name of the package that is a set of tools, not of the 
binaries.
The binaries that are called are gpasm, gpdasm, gplib, gplink
Comment 22 Chitlesh GOORAH 2006-09-24 14:24:40 EDT
(In reply to comment #16)
> Chitlesh,
> 
> I don't know if you saw that: http://ktechlab.org/download/gpsim.php
> 
> I commited the patch and I'll ask for rebuild on monday.
> 

Ok, then I'll upload ktechlab afterwards.

Mamuro, thanks for the approval.
For the moment, I don't have cvs access (blocked by my hostel's firewall)
I'll leave this bug open till, i commit it.

If I really need to commit it on the spot, do leave me a ping. I'll search
another internet connection. Thanks for understanding.

Comment 23 Chitlesh GOORAH 2006-09-24 14:25:52 EDT
This is what I'll upload:
SPEC: http://chitlesh.funpic.de/rpm/ktechlab.spec
SRPM: http://chitlesh.funpic.de/rpm/ktechlab-0.3-3.src.rpm
Comment 24 Alain Portal 2006-09-25 03:35:30 EDT
gpsim built is OK.
Comment 25 Mamoru TASAKA 2006-10-02 11:58:32 EDT
Chitlesh, importing this to CVS cannot be done yet?
Comment 26 Chitlesh GOORAH 2006-10-02 17:43:11 EDT
This friday I'm committing and building  it.
Comment 27 Chitlesh GOORAH 2006-10-09 10:50:42 EDT
Unfortunately, I wasn't unable to upload it last friday, but Ill do my best to
this friday.
Comment 28 Chitlesh GOORAH 2006-10-13 08:28:48 EDT
Created attachment 138422 [details]
build.log for ppc
Comment 29 Chitlesh GOORAH 2006-10-13 08:29:26 EDT
It failed for ppc. I need to look at it
see build.log for ppc
Comment 30 Mamoru TASAKA 2006-10-13 09:28:22 EDT
I just did a quick look and I found that some header files
required by src/electronics/port.cpp are not included in
glibc-headers-2.6.18-1.2784.fc6.ppc, so this is truly ppc
specific problem.
Comment 31 Mamoru TASAKA 2006-10-13 09:29:57 EDT
Oops, I meant "kernel-headers-2.6.18-1.2784.fc6.ppc" .
Comment 32 Alain Portal 2006-10-13 10:01:34 EDT
You just have to make a patch to remove the line "#include <sys/io.h>" in 
port.cpp and it will build fine.
See https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=185615

Unfortunately, I don't remember how to do to apply patch only on ppc arch.
Comment 33 Mamoru TASAKA 2006-10-13 10:15:22 EDT
Well, unfortunately, it seems for this case the relevant header files are:

#include <fcntl.h>
#include <sys/ioctl.h>
#include <sys/io.h>

All these header files are missing in kernel-headers.ppc .
I don't look into this yet precisely. Chitlesh, would you try if
compilation works if you remove all these includes when ppc?

(In reply to comment #32)
> Unfortunately, I don't remember how to do to apply patch only on ppc arch.
Perhaps there is a method like:

%ifarch ppc
%patch1 -p1 -b .header
%endif

Comment 34 Mamoru TASAKA 2006-10-13 11:02:28 EDT
Well, I tried to fix, however, again it failed...

http://buildsys.fedoraproject.org/logs/fedora-development-extras/19611-ktechlab-0.3-4.fc6/ppc/

In what file is ioctl() defined in ppc?
Comment 35 Alain Portal 2006-10-13 11:48:36 EDT
(In reply to comment #33)
> Well, unfortunately, it seems for this case the relevant header files are:
> 
> #include <fcntl.h>
> #include <sys/ioctl.h>

That's strange because these inclusions are in Pikdev and I did'nt get build 
failure about them.


Comment 36 Mamoru TASAKA 2006-10-13 11:57:35 EDT
(In reply to comment #32)
> You just have to make a patch to remove the line "#include <sys/io.h>" in 
> port.cpp and it will build fine.
> See https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=185615

Alain, you are right.
Chitlesh, please check the following.

http://buildsys.fedoraproject.org/logs/fedora-development-extras/19614-ktechlab-0.3-5.fc6/
http://buildsys.fedoraproject.org/logs/fedora-5-extras/19615-ktechlab-0.3-5.fc5/

Both (FE-devel, FE-5) succeeded. Closing this bug as CLOSED NEXTRELEASE.
Comment 37 Chitlesh GOORAH 2006-10-13 15:39:21 EDT
wow, thanks Mamuro
you did it before i had a look at it :)
Comment 38 Chitlesh GOORAH 2008-11-03 14:29:31 EST
Package Change Request
=======================
Package Name: ktechlab
Short Description: Development and simulation of microcontrollers and electronic circuits
Owners: chitlesh
Branches: EL-5
Comment 39 Mamoru TASAKA 2008-11-05 10:49:12 EST
(Closing as the last message is only new branch request.
 cvs flags can set set as ? even if the bug is left closed)
Comment 40 Toshio Ernie Kuratomi 2008-11-05 18:49:46 EST
cvs done.

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