This service will be undergoing maintenance at 00:00 UTC, 2017-10-23 It is expected to last about 30 minutes
Bug 1288823 - RFC: Ship libi2c
RFC: Ship libi2c
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: i2c-tools (Show other bugs)
25
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Adam Jackson
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2015-12-06 05:04 EST by Ruben Kerkhof
Modified: 2017-07-20 06:35 EDT (History)
6 users (show)

See Also:
Fixed In Version: i2c-tools-3.1.2-2
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2017-07-20 06:29:24 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Ruben Kerkhof 2015-12-06 05:04:33 EST
Collectd has a barometer plugin which uses libi2c.

Would it be possible to ship this library and the header?
Debian has a libi2c-dev package which does this.
Comment 1 Jan Kurik 2016-02-24 09:06:10 EST
This bug appears to have been reported against 'rawhide' during the Fedora 24 development cycle.
Changing version to '24'.

More information and reason for this action is here:
https://fedoraproject.org/wiki/Fedora_Program_Management/HouseKeeping/Fedora24#Rawhide_Rebase
Comment 2 Jan Kurik 2016-07-26 00:02:37 EDT
This bug appears to have been reported against 'rawhide' during the Fedora 25 development cycle.
Changing version to '25'.
Comment 3 Ondřej Lysoněk 2017-07-12 14:24:54 EDT
The problem is that the file /usr/include/linux/i2c-dev.h is already provided by kernel-headers. We could solve this by putting the file to e.g. /usr/include/i2c-tools/linux/i2c-dev.h, as suggested here:

https://fedoraproject.org/wiki/Packaging:Conflicts#Header_Name_Conflicts

You could then use something like
CFLAGS="$CFLAGS -I /usr/include/i2c-tools"
before calling configure in your spec file.

Will that work for you?
Comment 4 Jaroslav Škarvada 2017-07-12 18:42:02 EDT
(In reply to Ondřej Lysoněk from comment #3)
> The problem is that the file /usr/include/linux/i2c-dev.h is already
> provided by kernel-headers. We could solve this by putting the file to e.g.
> /usr/include/i2c-tools/linux/i2c-dev.h, as suggested here:
> 
> https://fedoraproject.org/wiki/Packaging:Conflicts#Header_Name_Conflicts
> 
> You could then use something like
> CFLAGS="$CFLAGS -I /usr/include/i2c-tools"
> before calling configure in your spec file.
> 
> Will that work for you?

The current state is a real mess, from the kernel Documentation/i2c/dev-interface [1]:

"The first thing to do is "#include <linux/i2c-dev.h>". Please note that
there are two files named "i2c-dev.h" out there, one is distributed
with the Linux kernel and is meant to be included from kernel
driver code, the other one is distributed with i2c-tools and is
meant to be included from user-space programs. You obviously want
the second one here."

I think it needs to be sorted out. I like the /usr/include/i2c-tools/linux location. Maybe also pkg-config could be provided. This could be all done downstream.

But maybe better to address the problem upstream, to both kernel (to update the doc) and to i2c-tools (to change the location of file). Simply the /usr/include/i2c-tools/i2c-dev.h could work in such case. Unfortunately, the i2c-tools upstream web page is dead (please also note we do not have the latest version of the package which is 3.1.2 and there is only 3.1.0 in the rawhide).

[1] https://www.kernel.org/doc/Documentation/i2c/dev-interface
Comment 5 Ruben Kerkhof 2017-07-13 06:18:09 EDT
Oh wow, I didn't realize this was such a mess.

Debian seems to move the kernel header out of the way when you install libi2c-dev:

root@stretch:~# apt-get install libi2c-dev
Reading package lists... Done
Building dependency tree
Reading state information... Done
Recommended packages:
  i2c-tools
The following NEW packages will be installed:
  libi2c-dev
0 upgraded, 1 newly installed, 0 to remove and 10 not upgraded.
Need to get 10.7 kB of archives.
After this operation, 30.7 kB of additional disk space will be used.
Get:1 http://deb.debian.org/debian stretch/main amd64 libi2c-dev all 3.1.2-3 [10.7 kB]
Fetched 10.7 kB in 0s (418 kB/s)
Selecting previously unselected package libi2c-dev.
(Reading database ... 21408 files and directories currently installed.)
Preparing to unpack .../libi2c-dev_3.1.2-3_all.deb ...
Adding 'diversion of /usr/include/linux/i2c-dev.h to /usr/include/linux/i2c-dev.h.kernel by libi2c-dev'
Unpacking libi2c-dev (3.1.2-3) ...
Setting up libi2c-dev (3.1.2-3) ...

I don't think that's really an option for us.

On the other hand, having the header in /usr/include/i2c-tools/linux will still cause issues depending on the order of the include path. If you do CFLAGS="$CFLAGS -I /usr/include/i2c-tools", headers from /usr/include might still be included first and we might pick up the wrong header.

My preference would be to rename the header, and install it in /usr/include/i2c, so client can do #include <i2c/i2c.h> and there's no conflict.
Comment 6 Ondřej Lysoněk 2017-07-13 07:06:46 EDT
So today I found the new upstream website [1] and it turns out they are already working on a solution to this [2]. They moved the additional code from /usr/include/linux/i2c-dev.h to /usr/include/i2c/smbus.h and moved the function definitions to .c files, making it a proper library.

However these changes happened in 2012 and have not yet appeared in a stable release, so who knows when it's actually coming. At first site the changes look complete, probably just need some testing. So I'm thinking maybe we could rebase to 3.1.2 and apply these patches in rawhide.

[1] https://i2c.wiki.kernel.org/index.php/I2C_Tools
[2] https://i2c.wiki.kernel.org/index.php/Plans_for_I2C_Tools_4
Comment 7 Jaroslav Škarvada 2017-07-13 07:17:09 EDT
(In reply to Ruben Kerkhof from comment #5)

> On the other hand, having the header in /usr/include/i2c-tools/linux will
> still cause issues depending on the order of the include path. If you do
> CFLAGS="$CFLAGS -I /usr/include/i2c-tools", headers from /usr/include might
> still be included first and we might pick up the wrong header.
> 
Yes, that's true, the header is including files from /usr/include/linux, so it needs to be renamed.

> My preference would be to rename the header, and install it in
> /usr/include/i2c, so client can do #include <i2c/i2c.h> and there's no
> conflict.

I would like more i2c-tools/i2c.h, but rename is the way to go.
Comment 8 Jaroslav Škarvada 2017-07-13 07:18:58 EDT
(In reply to Ondřej Lysoněk from comment #6)
> So today I found the new upstream website [1] and it turns out they are
> already working on a solution to this [2]. They moved the additional code
> from /usr/include/linux/i2c-dev.h to /usr/include/i2c/smbus.h and moved the
> function definitions to .c files, making it a proper library.
> 
> However these changes happened in 2012 and have not yet appeared in a stable
> release, so who knows when it's actually coming. At first site the changes
> look complete, probably just need some testing. So I'm thinking maybe we
> could rebase to 3.1.2 and apply these patches in rawhide.
> 
> [1] https://i2c.wiki.kernel.org/index.php/I2C_Tools
> [2] https://i2c.wiki.kernel.org/index.php/Plans_for_I2C_Tools_4

I think it's probably better to rebase and just rename now. I think the point is to enable the functionality in other packages (like collectd) with minimum patching.
Comment 9 Ruben Kerkhof 2017-07-13 07:26:11 EDT
> I would like more i2c-tools/i2c.h, but rename is the way to go.

Sure, I don't have a strong preference for the actual name, as long as it's not <linux/i2c-dev.h> :)
Comment 10 Ruben Kerkhof 2017-07-13 07:26:36 EDT
> I think it's probably better to rebase and just rename now. I think the point is to enable the functionality in other packages (like collectd) with minimum patching.

Sounds good!
Comment 11 Ondřej Lysoněk 2017-07-20 06:29:24 EDT
So I rebased it and put the header in /usr/include/i2c-tools in the i2c-tools-devel package, available in Rawhide. Keep in mind that this is temporary and it will change when i2c-tools version 4 comes out.
Comment 12 Ruben Kerkhof 2017-07-20 06:35:38 EDT
Thank you Ondřej. I'll patch Collectd to detect this header and make use of it.

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