Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libftdi: Incorrect pkg-config --cflags output #71623

Closed
1 task done
rojer opened this issue Feb 20, 2021 · 14 comments
Closed
1 task done

libftdi: Incorrect pkg-config --cflags output #71623

rojer opened this issue Feb 20, 2021 · 14 comments
Labels
bug Reproducible Homebrew/homebrew-core bug outdated PR was locked due to age upstream issue An upstream issue report is needed

Comments

@rojer
Copy link

rojer commented Feb 20, 2021

brew gist-logs <formula> link OR brew config AND brew doctor output

$ brew config
HOMEBREW_VERSION: 3.0.1-122-gcdaf5cc
ORIGIN: https://github.com/Homebrew/brew
HEAD: cdaf5cc
Last commit: 4 hours ago
Core tap ORIGIN: https://github.com/Homebrew/homebrew-core
Core tap HEAD: d87ba9a
Core tap last commit: 2 hours ago
Core tap branch: master
HOMEBREW_PREFIX: /usr/local
HOMEBREW_CASK_OPTS: []
HOMEBREW_EDITOR: vim
HOMEBREW_MAKE_JOBS: 16
Homebrew Ruby: 2.6.3 => /usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/2.6.3_2/bin/ruby
CPU: 16-core 64-bit kabylake
Clang: 12.0 build 1200
Git: 2.13.5 => /Library/Developer/CommandLineTools/usr/bin/git
Curl: 7.64.1 => /usr/bin/curl
macOS: 10.15.7-x86_64
CLT: 12.0.32.28
Xcode: 11.2 => /Applications/Xcode_11.2.0_beta_2_fb.app/Contents/Developer


  • The brew doctor above contains no "Warning" lines.

What were you trying to do (and why)?

use pkg-config --cflags libftdi1 to get cflags for libftdi

What happened (include all command output)?

cli/flash/cc3200/ftdi_libftdi1.go:26:10: fatal error: 'ftdi.h' file not found
#include <ftdi.h>
^~~~~~~~
1 error generated.

What did you expect to happen?

expected the project to compile like it used to

Step-by-step reproduction instructions (by running brew commands)

$ pkg-config --cflags libftdi1
-I/usr/local/Cellar/libftdi/1.5_1/include/libftdipp1 -I/usr/local/Cellar/libusb/1.0.24/include/libusb-1.0

@rojer rojer added the bug Reproducible Homebrew/homebrew-core bug label Feb 20, 2021
@rojer
Copy link
Author

rojer commented Feb 20, 2021

probably the result of #70229

looks like there's been a regression in the C version of the library: pkg-config produces the same cflags for libftdi1 and libftdipp1:

[rojer9@rojer9-mbp ~/mos master]$ pkg-config --cflags libftdi1
-I/usr/local/Cellar/libftdi/1.5_1/include/libftdipp1 -I/usr/local/Cellar/libusb/1.0.24/include/libusb-1.0
[rojer9@rojer9-mbp ~/mos master]$ pkg-config --cflags libftdipp1
-I/usr/local/Cellar/libftdi/1.5_1/include/libftdipp1 -I/usr/local/Cellar/libusb/1.0.24/include/libusb-1.0

that location has only c++ header:

[rojer9@rojer9-mbp ~/mos master]$ ls -l /usr/local/Cellar/libftdi/1.5_1/include/libftdipp1
total 16
-rw-r--r--  1 rojer9  staff  6574  7 Jul  2020 ftdi.hpp

in fact, the C header is stored in a different directory:

[rojer9@rojer9-mbp ~/mos master]$ ls -l /usr/local/Cellar/libftdi/1.5_1/include/libftdi1
total 48
-rw-r--r--  1 rojer9  staff  23611  7 Jul  2020 ftdi.h

ad a result, projects using pkg-config to get libftdi compilation flags fail to build.

rojer added a commit to mongoose-os/mos that referenced this issue Feb 20, 2021
@carlocab
Copy link
Member

#70229 isn't doing anything weird; it just enables a flag defined by the build system. It sounds like this isn't a Homebrew issue, and needs to be reported upstream instead.

@carlocab carlocab added the upstream issue An upstream issue report is needed label Feb 20, 2021
@rojer
Copy link
Author

rojer commented Feb 20, 2021

@carlocab possibly. but the homebrew change needs to be reverted first as it resulted in a regression to existing users.

@carlocab
Copy link
Member

carlocab commented Feb 20, 2021

I tend to view reverting changes as a last resort. We should try to fix it first, and that probably entails an upstream report.

In any case, reverting would require waiting for CI. Judging from the state of the job queue, that might take a few days.

@rojer
Copy link
Author

rojer commented Feb 20, 2021

while i agree with you in principle, precisely because it cannot be fixed within homebrew and requires upstream interaction, in my opinion we should go to last known good state, which is before the change. then we can deal with the upstream issue.

@rojer
Copy link
Author

rojer commented Feb 20, 2021

@xloem looks like enabling FTDI++ screws up cflags for libftdi1. if this can't be fixed quickly, please revert your change to unbreak the C library users.

@xloem
Copy link
Contributor

xloem commented Feb 20, 2021

I submitted the breaking change thinking I was helping. I don't have homebrew set up right now, but I'm looking at the libftdi source. In their CMakeLists.txt and *.pc.in, the same variable includedir is used in both pkgconfig templates, and it is indeed mutated by the option I enabled. A patch to libftdi to fix this would not be hard, but it is definitely a real issue.

I don't expect this library is used very much, because it duplicates functionality kernels usually already have, of accessing a serial device. I could be wrong.

EDIT: I don't know how to integrate a patch into your build system on like a 1-hour time schedule, reverting could make sense if this is a serious thing.

@rojer
Copy link
Author

rojer commented Feb 20, 2021

i applied a hack to get our project building again, so for us there is no immediate urgency. you could try fixing it by applying patch in homebrew first and then upstreaming it. use your judgement.

@xloem
Copy link
Contributor

xloem commented Feb 20, 2021

This issue was fixed in cdb28383402d248dbc6062f4391b038375c52385 on Jul 17, 2020 in upstream git.

@xloem
Copy link
Contributor

xloem commented Feb 20, 2021

The fix isn't in a release yet, here it is:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5aecafc..3b0b87c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -136,7 +136,7 @@ endif ()
 
 add_subdirectory(src)
 if ( FTDIPP )
-  project(libftdipp1 C CXX)
+  project(libftdi1 C CXX)
   add_subdirectory(ftdipp)
 endif ()
 if ( PYTHON_BINDINGS )

Do you guys need me to learn the packaging system better and submit another PR to add that patch?

@carlocab
Copy link
Member

Working on it now. Don't worry about it.

carlocab added a commit to carlocab/homebrew-core that referenced this issue Feb 20, 2021
@carlocab
Copy link
Member

carlocab commented Feb 20, 2021

Opened #71631.

As mentioned above, it may take a while to merge since the CI job queue is quite long at the moment.

@rojer
Copy link
Author

rojer commented Feb 20, 2021

thanks for the quick response, gentlemen!

@xloem
Copy link
Contributor

xloem commented Feb 21, 2021

my workaround for this bug was to reference ../libftdi1/ as the path, for compatibility

@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Mar 24, 2021
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Mar 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Reproducible Homebrew/homebrew-core bug outdated PR was locked due to age upstream issue An upstream issue report is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants