[fbthrift] Add new port #13072

curoky · 2020-08-22T03:58:59Z

Describe the pull request

What does your PR fix?
Add new port: facebook/fbthrift

Which triplets are supported/not supported? Have you updated the CI baseline?
N/A. No.

Does your PR follow the maintainer guide?
Yes.

curoky · 2020-08-22T17:29:40Z

Hello, how can i upgrade system package bison?

On osx, system default /usr/bin/bison is two old 2.5, but fbthrift need 3.0.4+.

CMake Error at /Volumes/data/downloads/tools/cmake-3.17.2-osx/cmake-3.17.2-Darwin-x86_64/CMake.app/Contents/share/cmake-3.17/Modules/FindPackageHandleStandardArgs.cmake:164 (message):
  Could NOT find BISON: Found unsuitable version "2.3", but required is at
  least "3.0.4" (found /usr/bin/bison)
Call Stack (most recent call first):
  /Volumes/data/downloads/tools/cmake-3.17.2-osx/cmake-3.17.2-Darwin-x86_64/CMake.app/Contents/share/cmake-3.17/Modules/FindPackageHandleStandardArgs.cmake:443 (_FPHSA_FAILURE_MESSAGE)
  /Volumes/data/downloads/tools/cmake-3.17.2-osx/cmake-3.17.2-Darwin-x86_64/CMake.app/Contents/share/cmake-3.17/Modules/FindBISON.cmake:296 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
  /Volumes/data/work/1/s/scripts/buildsystems/vcpkg.cmake:455 (_find_package)
  CMakeLists.txt:105 (find_package)

curoky · 2020-08-22T17:40:25Z

Azure-pipelines use brew to install bison, but brew don't symlink it into /usr/bin or /usr/local/bin.

vcpkg/scripts/azure-pipelines/osx/configuration/VagrantFile

Lines 17 to 21 in 299f581

    
           brew_formulas = [ 
        
             'autoconf', 
        
             'automake', 
        
             'libtool', 
        
             'bison' ]

$ brew install bison
...
bison is keg-only, which means it was not symlinked into /usr/local,
because macOS already provides this software and installing another version in
parallel can cause all kinds of trouble.

If you need to have bison first in your PATH run:
  echo 'export PATH="/usr/local/opt/bison/bin:$PATH"' >> ~/.zshrc

For compilers to find bison you may need to set:
  export LDFLAGS="-L/usr/local/opt/bison/lib"
...

So should i patch fbthrift, and then let cmake use the bison installed by brew.

find_package(BISON 3.0.4 REQUIRED)

to

find_package(BISON 3.0.4 REQUIRED PATHS /usr/local/opt/bison)

Is this the best solution?

ports/fbthrift/CONTROL

ports/fbthrift/portfile.cmake

NancyLi1013 · 2020-08-24T08:13:20Z

Hi @curoky
Thanks for this PR.
This port cannot build osx. Is it due to bsion?

curoky · 2020-08-24T12:40:55Z

Hi @curoky
Thanks for this PR.
This port cannot build osx. Is it due to bsion?

I'm not sure, that's just the first problem I encountered on osx.

JackBoosY · 2020-08-26T09:50:42Z

If this port need bison 3.0.4, please modify the following codes:

vcpkg/scripts/cmake/vcpkg_find_acquire_program.cmake

Lines 257 to 286 in 4a9b658

    
           elseif(VAR MATCHES "FLEX" OR VAR MATCHES "BISON") 
        
             if(CMAKE_HOST_WIN32) 
        
               set(SOURCEFORGE_ARGS 
        
                 REPO winflexbison 
        
                 FILENAME winflexbison-2.5.16.zip 
        
                 SHA512 0a14154bff5d998feb23903c46961528f8ccb4464375d5384db8c4a7d230c0c599da9b68e7a32f3217a0a0735742242eaf3769cb4f03e00931af8640250e9123 
        
                 NO_REMOVE_ONE_LEVEL 
        
                 WORKING_DIRECTORY "${DOWNLOADS}/tools/winflexbison" 
        
               ) 
        
               if(VAR MATCHES "FLEX") 
        
                 set(PROGNAME win_flex) 
        
               else() 
        
                 set(PROGNAME win_bison) 
        
               endif() 
        
               set(PATHS ${DOWNLOADS}/tools/winflexbison/0a14154bff-a8cf65db07) 
        
               if(NOT EXISTS "${PATHS}/data/m4sugar/m4sugar.m4") 
        
                 file(REMOVE_RECURSE "${PATHS}") 
        
               endif() 
        
             elseif(VAR MATCHES "FLEX") 
        
               set(PROGNAME flex) 
        
               set(APT_PACKAGE_NAME flex) 
        
               set(BREW_PACKAGE_NAME flex) 
        
             else() 
        
               set(PROGNAME bison) 
        
               set(APT_PACKAGE_NAME bison) 
        
               set(BREW_PACKAGE_NAME bison) 
        
               if (APPLE) 
        
                 set(PATHS /usr/local/opt/bison/bin) 
        
               endif() 
        
             endif()

JackBoosY · 2020-08-28T03:14:30Z

ports/fbthrift/0001-fix-bison-path-osx.patch

+ # Find required dependencies for thrift/compiler
+ if(compiler_only OR build_all)
+  if(APPLE)
+    set(BISON_EXECUTABLE "/usr/local/opt/bison/bin/bison" CACHE PATH "Bison executable")


Please don't do that.
Different bison versions will cause this port to fail to build. Therefore, we need to use the built-in bison in vcpkg instead of the system.

It seems that bison does not have a pre-compiled version, we need to compile bison from source during vcpkg installation.

@curoky We don't need to build bison now, as I said, we need to update vcpkg build-in tools bison instead.

I add these lines only for OSX, and only CMAKE_HOST_WIN32 have pre-compiled bison.

So how to use the built-in bison on OSX without compiling from source?

If there is no official pre-compiled version, or we don't want to compile from the source, then the version installed by brew is also a good choice.

$ brew info bison bison: stable 3.7.1 (bottled) [keg-only] Parser generator https://www.gnu.org/software/bison/ /usr/local/Cellar/bison/3.7.1 (94 files, 3.2MB) Poured from bottle on 2020-08-23 at 01:23:11 From: https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/bison.rb License: GPL-3.0 ==> Caveats bison is keg-only, which means it was not symlinked into /usr/local, because macOS already provides this software and installing another version in parallel can cause all kinds of trouble. If you need to have bison first in your PATH run: echo 'export PATH="/usr/local/opt/bison/bin:$PATH"' >> ~/.zshrc For compilers to find bison you may need to set: export LDFLAGS="-L/usr/local/opt/bison/lib"

vcpkg/scripts/cmake/vcpkg_find_acquire_program.cmake

Lines 258 to 275 in 4a9b658

if(CMAKE_HOST_WIN32)

set(SOURCEFORGE_ARGS

REPO winflexbison

FILENAME winflexbison-2.5.16.zip

SHA512 0a14154bff5d998feb23903c46961528f8ccb4464375d5384db8c4a7d230c0c599da9b68e7a32f3217a0a0735742242eaf3769cb4f03e00931af8640250e9123

NO_REMOVE_ONE_LEVEL

WORKING_DIRECTORY "${DOWNLOADS}/tools/winflexbison"

)

if(VAR MATCHES "FLEX")

set(PROGNAME win_flex)

else()

set(PROGNAME win_bison)

endif()

set(PATHS ${DOWNLOADS}/tools/winflexbison/0a14154bff-a8cf65db07)

if(NOT EXISTS "${PATHS}/data/m4sugar/m4sugar.m4")

file(REMOVE_RECURSE "${PATHS}")

endif()

elseif(VAR MATCHES "FLEX")

The bison path should be automatically added to the environment or general vcpkg_find_acquire_program is automatically exported, so there is no need to set its path here.
See

vcpkg/ports/libpcap/portfile.cmake

Lines 40 to 42 in 310f4df

vcpkg_find_acquire_program(BISON)

get_filename_component(BISON_PATH ${BISON} DIRECTORY)

vcpkg_add_to_path(${BISON_PATH})

I think the best way to handle this is to add this bison path (/usr/local/opt/bison/bin/bison) to be searched for in vcpkg_find_acquire_program(BISON) preferentially over the system bison on OSX. We can then pass this in directly to the build using OPTIONS -DBISON_EXECUTABLE=${BISON} as an additional argument to vcpkg_configure_cmake().

@ras0219 👍🏻️ Thanks for your suggestion, it's elegant and simple.

JackBoosY · 2020-09-03T08:26:32Z

ports/fbthrift/0001-fix-bison-path-osx.patch

+ # Find required dependencies for thrift/compiler
+ if(compiler_only OR build_all)
+  if(APPLE)
+    set(BISON_EXECUTABLE "/usr/local/opt/bison/bin/bison" CACHE PATH "Bison executable")


The bison path should be automatically added to the environment or general vcpkg_find_acquire_program is automatically exported, so there is no need to set its path here.
See

vcpkg/ports/libpcap/portfile.cmake

Lines 40 to 42 in 310f4df

vcpkg_find_acquire_program(BISON)

get_filename_component(BISON_PATH ${BISON} DIRECTORY)

vcpkg_add_to_path(${BISON_PATH})

curoky · 2020-09-12T12:23:56Z

@JackBoosY Also thanks for your suggestion. I just updated this pr. Hope this one can go through soon.

ras0219-msft · 2020-09-28T01:55:56Z

Thanks for the PR!

curoky added 2 commits August 23, 2020 01:14

[fbthrift] Add new port

fd93f8f

[fbthrift] only supports x64, except osx

5c469a9

NancyLi1013 self-assigned this Aug 24, 2020

NancyLi1013 suggested changes Aug 24, 2020

View reviewed changes

NancyLi1013 added category:new-port The issue is requesting a new library to be added; consider making a PR! requires:author-response labels Aug 24, 2020

curoky added 3 commits August 25, 2020 22:02

[fbthrift] add missing vcpkg_fail_port_install

207870e

[fbthrift] format code

8a860b0

[fbthrift] use vcpkg_copy_tools install thrift1

fd3e5c0

NancyLi1013 approved these changes Aug 26, 2020

View reviewed changes

NancyLi1013 removed the requires:author-response label Aug 26, 2020

JackBoosY added the requires:author-response label Aug 26, 2020

[fbthrift] support build on osx

a299d66

JackBoosY suggested changes Aug 28, 2020

View reviewed changes

JackBoosY suggested changes Sep 3, 2020

View reviewed changes

[fbthrift] specify the bison path directly

d7af411

ras0219 approved these changes Sep 13, 2020

View reviewed changes

NancyLi1013 added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Sep 14, 2020

JackBoosY approved these changes Sep 14, 2020

View reviewed changes

ras0219-msft merged commit 10d005d into microsoft:master Sep 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fbthrift] Add new port #13072

[fbthrift] Add new port #13072

curoky commented Aug 22, 2020

curoky commented Aug 22, 2020

curoky commented Aug 22, 2020

NancyLi1013 commented Aug 24, 2020

curoky commented Aug 24, 2020 •

edited

Loading

JackBoosY commented Aug 26, 2020

JackBoosY Aug 28, 2020

curoky Aug 31, 2020

JackBoosY Aug 31, 2020

curoky Aug 31, 2020

JackBoosY Sep 3, 2020

ras0219 Sep 3, 2020 •

edited

Loading

curoky Sep 12, 2020

JackBoosY Sep 3, 2020

curoky commented Sep 12, 2020

ras0219-msft commented Sep 28, 2020

	if(CMAKE_HOST_WIN32)
	set(SOURCEFORGE_ARGS
	REPO winflexbison
	FILENAME winflexbison-2.5.16.zip
	SHA512 0a14154bff5d998feb23903c46961528f8ccb4464375d5384db8c4a7d230c0c599da9b68e7a32f3217a0a0735742242eaf3769cb4f03e00931af8640250e9123
	NO_REMOVE_ONE_LEVEL
	WORKING_DIRECTORY "${DOWNLOADS}/tools/winflexbison"
	)
	if(VAR MATCHES "FLEX")
	set(PROGNAME win_flex)
	else()
	set(PROGNAME win_bison)
	endif()
	set(PATHS ${DOWNLOADS}/tools/winflexbison/0a14154bff-a8cf65db07)
	if(NOT EXISTS "${PATHS}/data/m4sugar/m4sugar.m4")
	file(REMOVE_RECURSE "${PATHS}")
	endif()
	elseif(VAR MATCHES "FLEX")

	vcpkg_find_acquire_program(BISON)
	get_filename_component(BISON_PATH ${BISON} DIRECTORY)
	vcpkg_add_to_path(${BISON_PATH})

[fbthrift] Add new port #13072

[fbthrift] Add new port #13072

Conversation

curoky commented Aug 22, 2020

curoky commented Aug 22, 2020

curoky commented Aug 22, 2020

NancyLi1013 commented Aug 24, 2020

curoky commented Aug 24, 2020 • edited Loading

JackBoosY commented Aug 26, 2020

JackBoosY Aug 28, 2020

Choose a reason for hiding this comment

curoky Aug 31, 2020

Choose a reason for hiding this comment

JackBoosY Aug 31, 2020

Choose a reason for hiding this comment

curoky Aug 31, 2020

Choose a reason for hiding this comment

JackBoosY Sep 3, 2020

Choose a reason for hiding this comment

ras0219 Sep 3, 2020 • edited Loading

Choose a reason for hiding this comment

curoky Sep 12, 2020

Choose a reason for hiding this comment

JackBoosY Sep 3, 2020

Choose a reason for hiding this comment

curoky commented Sep 12, 2020

ras0219-msft commented Sep 28, 2020

curoky commented Aug 24, 2020 •

edited

Loading

ras0219 Sep 3, 2020 •

edited

Loading