Re: [Gitorious] Activity: dschanoeh updated merge request for...

Re: [Gitorious] Activity: dschanoeh updated merge request for...

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1559 bytes --]

Hello * and dschanoeh,

On 11/29/2011 01:14 PM, Gitorious wrote:
> ------------------------------------------------------------------------
> dschanoeh updated merge request for dschanoehs-can-utils with can-utils.
> The merge request is at http://gitorious.org/linux-can/can-utils/merge_requests/1
> ------------------------------------------------------------------------

I just received this pull request from you. It basically consists of
* removing plain old Makefile
* fixes for the old include directory

Regarding removing Makefile, fine with me, Oliver what do you think?

I've started some time ago to import the includes from the svn into the
git, but kernel only includes were still left. Now I just updated my git
repo which merges the includes and adopts both Makefile and autotools.

There is a squashed version, which imports the includes without their
history. This branch is called master-squashed.

Please checkout the master branch (or master-squashed) of my repo and test:
https://gitorious.org/~bet-frogger/linux-can/mkl-can-utils

There is a j1939 branch, too.

Please test and comment, if everything works I want to commit either my
master branch or master-squashed to the official repo, which branch is
better?

cheers, Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Gitorious] Activity: dschanoeh updated merge request for...

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 3710 bytes --]

Hello Jan-Niklas,

please don't top-post.

On 12/03/2011 05:47 PM, Jan-Niklas Meier wrote:
> 1.
> I think adding the includes to the repository is a bad idea. If you compile
> the utils for your machine which has a kernel with SocketCAN-support why
> not use the original kernel headers?

Several reasons:
- iputils2 package the kernel headers, too.
- Consider you're using an older kernel without official socketcan
  support and with the external kernel modules available on gitorious.
- What about isotp and/or j1939 support? You want to use new features
  which are not part of the mainline kernel yet.

> It makes no sense to compile the can-utils on a machine which does not have
> SocketCAN support. If you do so you should be cross-compiling for a
> different machine and have the kernel headers of this machine in your
> cross-environment.
> Also it is very unlikely that someone who installed make, gcc and the
> autotools does not also have his kernel includes, right?

> To be able to compile a subset of the tools I checked if the isotp.h and
> gw.h are present in the include directory (I don't have them both) and only
> compile the corresponding utils if the headers are present. This way it
> should still be possible to compile subsets of the tools on machines with
> old kernels and not ending up with tools you can not use afterwards.

What about the external kernel modules use case?

> 2.
> I tried to rewrite the autoconf.ac because there are macros I'm having

Please don't rewrite stuff, please send incremental patches, fixing one
problem at a time.

> trouble with. When I run autoconf (version 2.68, libtool and automake
> installed) I get the following output:
> 
> configure.ac:22: error: possibly undefined macro: AC_LIBTOOL_WIN32_DLL
>       If this token and others are legitimate, please use m4_pattern_allow.
>       See the Autoconf documentation.
> configure.ac:24: error: possibly undefined macro: AC_PROG_LIBTOOL
> configure.ac:28: error: possibly undefined macro: AM_INIT_AUTOMAKE
> 
> A configure script is produced but when I try to execute it I get (shtool
> is installed):
> 
> configure: error: cannot find install-sh, install.sh, or shtool in
> config/autoconf "."/config/autoconf
> 
> Trying to make it work (and not knowing what the macros do anyway ;) I
> removed them and successfully ran autoconf with my configure.ac...
                                    ^^^^^^^^

That might be the problem, don't run autoconf, run $(./autogen.sh) instead.

I just updated to the latest tool available on debian unstable and it works:

> [frogger@hardanger:can-utils (master-squashed)]$ autoconf --version
> autoconf (GNU Autoconf) 2.68
[..]

> automake (GNU automake) 1.11.1
[..]

> [frogger@hardanger:can-utils (master-squashed)]$ libtoolize --version
> libtoolize (GNU libtool) 2.4.2
[..]

> [frogger@hardanger:can-utils (master-squashed)]$ ./autogen.sh && ./configure && make distcheck
[...]
> =================================================
> can-utils-trunk archives ready for distribution:
> can-utils-trunk.tar.gz
> can-utils-trunk.tar.bz2
> =================================================

I used the master-squashed branch of my can-utils repo:
https://gitorious.org/~bet-frogger/linux-can/mkl-can-utils, head is:

231ed32f497a4f403a6d0ce2a61f1bb94be76ea4 gitignore: added tarballs

cheers, Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Gitorious] Activity: dschanoeh updated merge request for...

[Jan-Niklas Meier]

2011/12/3 Marc Kleine-Budde <mkl@pengutronix.de>
>
> Hello Jan-Niklas,
>
> please don't top-post.

I'm sorry.

>
> On 12/03/2011 05:47 PM, Jan-Niklas Meier wrote:
> > 1.
> > I think adding the includes to the repository is a bad idea. If you compile
> > the utils for your machine which has a kernel with SocketCAN-support why
> > not use the original kernel headers?
>
> Several reasons:
> - iputils2 package the kernel headers, too.
> - Consider you're using an older kernel without official socketcan
>  support and with the external kernel modules available on gitorious.
> - What about isotp and/or j1939 support? You want to use new features
>  which are not part of the mainline kernel yet.
>
> > It makes no sense to compile the can-utils on a machine which does not have
> > SocketCAN support. If you do so you should be cross-compiling for a
> > different machine and have the kernel headers of this machine in your
> > cross-environment.
> > Also it is very unlikely that someone who installed make, gcc and the
> > autotools does not also have his kernel includes, right?
>
> > To be able to compile a subset of the tools I checked if the isotp.h and
> > gw.h are present in the include directory (I don't have them both) and only
> > compile the corresponding utils if the headers are present. This way it
> > should still be possible to compile subsets of the tools on machines with
> > old kernels and not ending up with tools you can not use afterwards.
>
> What about the external kernel modules use case?

Thank you very much for the clarification. Now I understand why you
did this. Still if your kernel's CAN API is much older than the
headers included in the current can-utils the programs will compile
fine but it is possible that some of them will fail at runtime right?
Method signatures could have changed or some methods are not present
in the old kernel. For example using a program with some specific
parameters could lead to strange behavior for the user. Is that true
or am I missing something?

>
> > 2.
> > I tried to rewrite the autoconf.ac because there are macros I'm having
>
> Please don't rewrite stuff, please send incremental patches, fixing one
> problem at a time.
>
> > trouble with. When I run autoconf (version 2.68, libtool and automake
> > installed) I get the following output:
> >
> > configure.ac:22: error: possibly undefined macro: AC_LIBTOOL_WIN32_DLL
> >       If this token and others are legitimate, please use m4_pattern_allow.
> >       See the Autoconf documentation.
> > configure.ac:24: error: possibly undefined macro: AC_PROG_LIBTOOL
> > configure.ac:28: error: possibly undefined macro: AM_INIT_AUTOMAKE
> >
> > A configure script is produced but when I try to execute it I get (shtool
> > is installed):
> >
> > configure: error: cannot find install-sh, install.sh, or shtool in
> > config/autoconf "."/config/autoconf
> >
> > Trying to make it work (and not knowing what the macros do anyway ;) I
> > removed them and successfully ran autoconf with my configure.ac...
>                                    ^^^^^^^^
>
> That might be the problem, don't run autoconf, run $(./autogen.sh) instead.

\o/ Yay! That's the hint I needed. I didn't know that there is a
different way to generate a ./configure that autoconf.

>
>
> I just updated to the latest tool available on debian unstable and it works:
>
> > [frogger@hardanger:can-utils (master-squashed)]$ autoconf --version
> > autoconf (GNU Autoconf) 2.68
> [..]
>
> > automake (GNU automake) 1.11.1
> [..]
>
> > [frogger@hardanger:can-utils (master-squashed)]$ libtoolize --version
> > libtoolize (GNU libtool) 2.4.2
> [..]
>
> > [frogger@hardanger:can-utils (master-squashed)]$ ./autogen.sh && ./configure && make distcheck
> [...]
> > =================================================
> > can-utils-trunk archives ready for distribution:
> > can-utils-trunk.tar.gz
> > can-utils-trunk.tar.bz2
> > =================================================
>
> I used the master-squashed branch of my can-utils repo:
> https://gitorious.org/~bet-frogger/linux-can/mkl-can-utils, head is:
>
> 231ed32f497a4f403a6d0ce2a61f1bb94be76ea4 gitignore: added tarballs

I did the same and it works fine with both HEADs of your master and
master-squashed branches. Still I get some warnings during autoreconf:

configure.ac:22: warning: The macro `AC_LIBTOOL_WIN32_DLL' is obsolete.
configure.ac:22: You should run autoupdate.
[...]
configure.ac:24: warning: The macro `AC_PROG_LIBTOOL' is obsolete.
configure.ac:24: You should run autoupdate.

Thank you again!
Jan-Niklas

Re: [Gitorious] Activity: dschanoeh updated merge request for...

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 2827 bytes --]

On 12/04/2011 10:57 AM, Jan-Niklas Meier wrote:
>>> To be able to compile a subset of the tools I checked if the isotp.h and
>>> gw.h are present in the include directory (I don't have them both) and only
>>> compile the corresponding utils if the headers are present. This way it
>>> should still be possible to compile subsets of the tools on machines with
>>> old kernels and not ending up with tools you can not use afterwards.
>>
>> What about the external kernel modules use case?
> 
> Thank you very much for the clarification. Now I understand why you
> did this. Still if your kernel's CAN API is much older than the
> headers included in the current can-utils the programs will compile
> fine but it is possible that some of them will fail at runtime right?

Yes. If the kernel doesn't implement the features your userspace program
needs.

> Method signatures could have changed or some methods are not present
> in the old kernel. For example using a program with some specific
> parameters could lead to strange behavior for the user. Is that true
> or am I missing something?

Yes - you're missing the policy. The Kernel-Userspace interface must not
be changed in an incompatible way, at least once an interface is in the
mainline kernel. This includes not only the API but also the ABI (the
binary interface) to the kernel. This rule is (next to) 100% hard for
system calls, ioctls, etc... modern things like the sysfs tend to change
sometimes.

For example, the isotp programs will fail with "Protocol not supported"
during socket creation.

>>> I tried to rewrite the autoconf.ac because there are macros I'm having

[...]

>> That might be the problem, don't run autoconf, run $(./autogen.sh) instead.
> 
> \o/ Yay! That's the hint I needed. I didn't know that there is a
> different way to generate a ./configure that autoconf.

[...]

> I did the same and it works fine with both HEADs of your master and
> master-squashed branches. Still I get some warnings during autoreconf:
> 
> configure.ac:22: warning: The macro `AC_LIBTOOL_WIN32_DLL' is obsolete.
> configure.ac:22: You should run autoupdate.
> [...]
> configure.ac:24: warning: The macro `AC_PROG_LIBTOOL' is obsolete.
> configure.ac:24: You should run autoupdate.

These are deprecation warnings from the autotools. IIRC these come from
libtool-2.x, but you need them for pre pre 2.x to work. Debian lenny
(this is old-stable, isn't it?) uses still libtool-1.5. Maybe some old
redhat versions, too.

cheers, Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Gitorious] Activity: dschanoeh updated merge request for...

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 837 bytes --]

On 12/04/2011 10:57 AM, Jan-Niklas Meier wrote:
[...]

>> I used the master-squashed branch of my can-utils repo:
>> https://gitorious.org/~bet-frogger/linux-can/mkl-can-utils, head is:
>>
>> 231ed32f497a4f403a6d0ce2a61f1bb94be76ea4 gitignore: added tarballs
> 
> I did the same and it works fine with both HEADs of your master and
> master-squashed branches. Still I get some warnings during autoreconf:

May I add your Tested-by: to the patche series I posted on the 2nd of
december ("[PATCH 0/4] import kernel header files into tree").

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Gitorious] Activity: dschanoeh updated merge request for...

[Jan-Niklas Meier]

2011/12/5 Marc Kleine-Budde <mkl@pengutronix.de>:
> [...]
>
> May I add your Tested-by: to the patche series I posted on the 2nd of
> december ("[PATCH 0/4] import kernel header files into tree").
>

Yes you can :)