Re: [PATCH] CMake: move options to a dedicated file
On 02/20/2018 12:56 PM, Yegor Yefremov wrote: > You're right ftdi_eeprom should be enabled by default. Python bindings > have more dependencies so I would let it OFF by default. ok, sounds like a good compromise for now. I've committed a change for this. >> What about having libconfuse / boost library detection >> in CMakeOptions.txt to set the default values? > > I'm using libftdi in the Buildroot context. In this case it is > important/desired to have predictable, reproducible builds. If I > configure the option FTDI_EEPROM as OFF, then I expect that I get no > ftdi_eeprom binary even if I have libconfuse. > > Another approach would be NOT to define FTDI_EEPROM as an option, but > just as a variable, that depends on whether libconfuse could be found > or not. In this case the behavior is controlled externally through the > development environment. > > But I'm more for the first variant. yes, the first variant is more obvious. Your patch has landed on master :) Cheers, Thomas -- libftdi - see http://www.intra2net.com/en/developer/libftdi for details. To unsubscribe send a mail to libftdi+unsubscr...@developer.intra2net.com
Re: [PATCH] CMake: move options to a dedicated file
Hi Thomas, sorry for delay. On Mon, Feb 5, 2018 at 11:36 PM, Thomas Jaroschwrote: > Hi Yegor, > > On 01/03/2018 01:46 PM, Yegor Yefremov wrote: >> Also disable all options having extra dependencies. >> >> If an option is selected makes its dependencies as REQUIRED. > > thanks for the big patch. It's almost merged :) > > What needs a bit of discussion is the change in default behavior: > > The old cmake script was "best effort": If you f.e. had libconfuse > installed, ftdi_eeprom was automatically build. > > With the new way of doing things, ftdi_eeprom needs to be enabled upon > request. This changes user expectations that have grown over many years. > > Do you think we can re-enable ftdi_eeprom > and the python bindings by default? > Not sure how widespread the C++ wrapper is used. You're right ftdi_eeprom should be enabled by default. Python bindings have more dependencies so I would let it OFF by default. > What about having libconfuse / boost library detection > in CMakeOptions.txt to set the default values? I'm using libftdi in the Buildroot context. In this case it is important/desired to have predictable, reproducible builds. If I configure the option FTDI_EEPROM as OFF, then I expect that I get no ftdi_eeprom binary even if I have libconfuse. Another approach would be NOT to define FTDI_EEPROM as an option, but just as a variable, that depends on whether libconfuse could be found or not. In this case the behavior is controlled externally through the development environment. But I'm more for the first variant. As for Boost as long as it is mostly interesting for testing, it is a more a matter of configuring the CI server. Yegor -- libftdi - see http://www.intra2net.com/en/developer/libftdi for details. To unsubscribe send a mail to libftdi+unsubscr...@developer.intra2net.com
Re: [PATCH] CMake: move options to a dedicated file
Hi Yegor, On 01/03/2018 01:46 PM, Yegor Yefremov wrote: > Also disable all options having extra dependencies. > > If an option is selected makes its dependencies as REQUIRED. thanks for the big patch. It's almost merged :) What needs a bit of discussion is the change in default behavior: The old cmake script was "best effort": If you f.e. had libconfuse installed, ftdi_eeprom was automatically build. With the new way of doing things, ftdi_eeprom needs to be enabled upon request. This changes user expectations that have grown over many years. Do you think we can re-enable ftdi_eeprom and the python bindings by default? Not sure how widespread the C++ wrapper is used. What about having libconfuse / boost library detection in CMakeOptions.txt to set the default values? Cheers, Thomas -- libftdi - see http://www.intra2net.com/en/developer/libftdi for details. To unsubscribe send a mail to libftdi+unsubscr...@developer.intra2net.com