gordonklaus / ineffassign

Detect ineffectual assignments in Go code.
MIT License
397 stars 23 forks source link

False positive or explain: could not import C (no metadata for C) #67

Closed mbideau closed 3 years ago

mbideau commented 3 years ago

Hi, and first thank you for this program.

I have run a Go Report on one of my project, and your program is outputting the following warning:

btrfs-diff-go/pkg/btrfs-diff.go
        Line 16: warning: could not import C (no metadata for C) (ineffassign)

It is related to the following line :

package btrfsdiff

// We get the constants from this header.

// #include <btrfs/send.h>
// #include <btrfs/send-utils.h>
// #include <btrfs/ioctl.h>
// #cgo LDFLAGS: -lbtrfs
import "C"

And then the C import is used like in the following lines :

var commandsDefs *[C.__BTRFS_SEND_C_MAX]commandType = initCommandsDefinitions()

Can you explain to me why the warning, or try to fix it, if it is a false positive.

Thank you in advance.

If I can be of any help, let me know.

gordonklaus commented 3 years ago

Hi @mbideau

It's not a false positive (nor a true positive), but rather a problem loading your package. We're using golang.org/x/tools/go/packages to load packages, and this is how it complains when the btrfs C library is missing. I think you will find that if you run ineffassign locally where you have libbtrfs-dev installed, you will not see this error. On goreportcard.com's servers, of course, the C library may not be present.

I will try to fix ineffassign. It is currently loading type information, which it does not need. If we only load syntax, then the C library does not need to be present.

On your end, you could fix the problem by including the btrfs C headers and library in your repository. This has the added benefit that users of your package don't need to install any dependencies, and you have full control of the C library version.

mbideau commented 3 years ago

Hi @gordonklaus,

Thank you very much for your explanation.

I should have thought about it (that you might not have sources for libbtrfs) :sweat_smile:

I can confirm that locally ineffassign have no issue at all :+1:

I don't know how to embed in my repo the required dependencies and make go use them ? For information, that's what I have on my computer (Debian buster) after a fresh build :

❯ ldd ./btrfs-diff-go 
    linux-vdso.so.1 (0x00007ffec6d8f000)
    libbtrfs.so.0 => /usr/lib/x86_64-linux-gnu/libbtrfs.so.0 (0x00007f2addeff000)
    libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f2addede000)
    libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f2addd1d000)
    libuuid.so.1 => /lib/x86_64-linux-gnu/libuuid.so.1 (0x00007f2addd14000)
    libblkid.so.1 => /lib/x86_64-linux-gnu/libblkid.so.1 (0x00007f2addcbf000)
    libmount.so.1 => /lib/x86_64-linux-gnu/libmount.so.1 (0x00007f2addc60000)
    /lib64/ld-linux-x86-64.so.2 (0x00007f2addf74000)
    libselinux.so.1 => /lib/x86_64-linux-gnu/libselinux.so.1 (0x00007f2adda36000)
    librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007f2adda2c000)
    libpcre.so.3 => /lib/x86_64-linux-gnu/libpcre.so.3 (0x00007f2add9b8000)
    libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f2add9b3000)

Do you have any lead for me ? That would be great :smiley:

Thank you in advance :pray:

PS: closing the issue since it is not one

gordonklaus commented 3 years ago

To embed the btrfs dependencies in your repo, you would include the header and library files right in the repo, for example in a btrfs directory next to btrfs-diff.go. You might have to update the #include directives (preceding import "C") to match the local path to the headers, if you use a different path. I guess you'll also have to update LDFLAGS similarly, adding a -L./lib option, or something, so it can find the local library. And of course you'll need to include the static library (libbtrfs.a) so that it is statically linked.

(You probably want to uninstall libbtrfs-dev when trying this out, or Go might accidentally find the system headers and libraries instead of the local ones.)

Sorry I can't give more specific advice. I haven't actually taken this approach myself. I thought I'd heard of others doing it, but I can't find any examples now.