Skip to content

Conversation

@lidavidm
Copy link
Member

@lidavidm lidavidm commented Jun 8, 2022

gRPC doesn't support building with certain flags that distros insert

@github-actions
Copy link

github-actions bot commented Jun 8, 2022

@github-actions
Copy link

github-actions bot commented Jun 8, 2022

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@lidavidm
Copy link
Member Author

lidavidm commented Jun 8, 2022

@github-actions crossbow submit github-ubuntu-focal-amd64 travis-ubuntu-focal-arm64

@github-actions
Copy link

github-actions bot commented Jun 8, 2022

Unable to match any tasks for `github-ubuntu-focal-amd64`
The Archery job run can be found at: https://github.com/apache/arrow/actions/runs/2463986777

@lidavidm
Copy link
Member Author

lidavidm commented Jun 8, 2022

@github-actions crossbow submit focal

@github-actions
Copy link

github-actions bot commented Jun 8, 2022

Revision: 66f00f3

Submitted crossbow builds: ursacomputing/crossbow @ actions-4fe9ae9e94

Task Status
test-r-rstudio-r-base-4.2-focal Azure
ubuntu-focal-amd64 Github Actions
ubuntu-focal-arm64 TravisCI

@lidavidm lidavidm requested a review from kou June 8, 2022 21:38
Comment on lines 3666 to 3667
string(REPLACE "-Wformat-security" "" GRPC_C_FLAGS "${GRPC_C_FLAGS}")
string(REPLACE "-Wformat-security" "" GRPC_CXX_FLAGS "${GRPC_CXX_FLAGS}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need them? I think that -Wno-format-security wins with -Wformat-security -Wno-format-security. (-Wformat-security is ignored.)

Comment on lines 3662 to 3665
set(GRPC_C_FLAGS
"${EP_C_FLAGS} -Wno-attributes -Wno-format-security -Wno-unknown-warning-option")
set(GRPC_CXX_FLAGS
"${EP_CXX_FLAGS} -Wno-attributes -Wno-format-security -Wno-unknown-warning-option")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add these flags in if(NOT MSVC) ... endif()?

@lidavidm
Copy link
Member Author

lidavidm commented Jun 8, 2022

@github-actions crossbow submit ubuntu-focal-*

set(GRPC_CXX_FLAGS
"${EP_CXX_FLAGS} -Wno-attributes -Wno-format-security -Wno-unknown-warning-option"
)
endif()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to initialize GRPC_C*_FLAGS for MSVC.

e.g.:

set(GRPC_C_FLAGS "${EP_C_FLAGS}")
set(GRPC_CXX_FLAGS "${EP_CXX_FLAGS}")
if(NOT MSVC)
  set(GRPC_C_FLAGS "${GRPC_C_FLAGS} ...")
  set(GRPC_CXX_FLAGS "${GRPC_CXX_FLAGS} ...")
endif()

Or

set(GRPC_CMAKE_ARGS ...)
if(NOT MSVC)
  set(GRPC_C_FLAGS "${EP_C_FLAGS} ...")
  set(GRPC_CXX_FLAGS "${EP_CXX_FLAGS} ...")
  list(APPEND GRPC_CMAKE_ARGS "-DCMAKE_C_FLAGS=${GRPC_C_FLAGS}" ...)
endif()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

D'oh…thanks for catching that

@github-actions
Copy link

github-actions bot commented Jun 8, 2022

Revision: 4628035

Submitted crossbow builds: ursacomputing/crossbow @ actions-98cbb695da

Task Status
ubuntu-focal-amd64 Github Actions
ubuntu-focal-arm64 TravisCI

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants