unbreak getdeps build by pepeiborra · Pull Request #160 · facebookincubator/hsthrift

pepeiborra · 2025-07-18T15:31:46Z

Summary: Make getdeps work again

Differential Revision: D78560675

facebook-github-bot · 2025-07-18T15:31:59Z

This pull request was exported from Phabricator. Differential Revision: D78560675

simonmar · 2025-07-19T00:18:30Z


 THRIFT1 := thrift1
-CABAL := $(CABAL_BIN) $(CABAL_CONFIG_FLAGS) $(GETDEPS_CABAL_FLAGS)
+CABAL := $(CABAL_BIN) $(CABAL_CONFIG_FLAGS) $(GETDEPS_CABAL_FLAGS) -f-bundled-folly -f-fbthrift


Don't set flags here. They're supposed to be set in CABAL_CONFIG_FLAGS, which is set by the CI workflow, or cabal.project.

Also, fbthrift should be enabled for the getdeps build

Don't set flags here. They're supposed to be set in CABAL_CONFIG_FLAGS, which is set by the CI workflow, or cabal.project.

Cannot easily set them in the internal CI workflow, already tried that. Re cabal.project, isn't it shared with the GitHub CI workflow?

Also, fbthrift should be enabled for the getdeps build

The fbthrift build with getdeps is broken since the folly-clib PR

Last I checked the breakage was in fbthrift itself, CI was passing on the PR when it was landed.

true, but fbthrift itself is fixed now

The current error is cabal complaining about a leading zero in the folly-clib version number, I can put up a fix for that. https://github.com/facebookincubator/hsthrift/actions/runs/16402006410/job/46342785108

This PR won't work because the cabal CI job will fail - it needs bundled-folly to be enabled: https://github.com/facebookincubator/hsthrift/actions/runs/16374421038/job/46270656525?pr=160

Maybe it's possible for me to run the getdeps build locally, I haven't tried that yet.

@pepeiborra what's the command to run the getdeps build? I put it on the internal wiki somewhere but I don't remember what it was exactly. There were some extra flags IIRC.

We should probably have a github CI job that does the build via getdeps, to match the internal CI job.

getdeps.py build hsthrift --allow-system-packages --shared-libs --extra-cmake-def
ines='{"BOOST_LINK_STATIC": "OFF"}'

facebook-github-bot · 2025-07-22T09:25:39Z

This pull request was exported from Phabricator. Differential Revision: D78560675

Summary: Pull Request resolved: facebookincubator#160 Make getdeps work again Reviewed By: kbojarczuk Differential Revision: D78560675

simonmar · 2025-07-22T12:37:48Z

+package folly-clib
+  flags: -bundled-folly
+package thrift-tests
+  flags: -fbthrift


why is this needed? I would hope the fbthrift tests should pass, since we have fbthrift.

cabal build fails

simonmar · 2025-07-22T12:39:12Z

@@ -0,0 +1,23 @@
+-- Copyright (c) Facebook, Inc. and its affiliates.
+
+packages:


might be able to avoid the copy/paste by using include

leaving that for future work

facebook-github-bot · 2025-07-22T15:28:55Z

This pull request was exported from Phabricator. Differential Revision: D78560675

Summary: Pull Request resolved: facebookincubator#160 Make getdeps work again Reviewed By: kbojarczuk Differential Revision: D78560675

simonmar · 2025-07-23T09:48:34Z

We should have a default cabal.project, so that the build instructions work.

facebook-github-bot · 2025-07-23T14:10:30Z

This pull request was exported from Phabricator. Differential Revision: D78560675

Summary: Pull Request resolved: facebookincubator#160 Make getdeps work again Reviewed By: kbojarczuk Differential Revision: D78560675

facebook-github-bot · 2025-07-24T10:00:31Z

This pull request was exported from Phabricator. Differential Revision: D78560675

simonmar · 2025-07-24T11:39:00Z

FYI I made a CI job that builds using getdeps: simonmar@b224d79

it's not quite ready yet, but it works.

Summary: X-link: facebookincubator/hsthrift#160 Make getdeps work again Reviewed By: kbojarczuk Differential Revision: D78560675 fbshipit-source-id: 7602f45c02eecda9ec5bda9b32a9b0d5d23766b7

facebook-github-bot · 2025-07-25T10:32:35Z

This pull request has been merged in 76d157f.

Summary: X-link: facebookincubator/hsthrift#160 Make getdeps work again Reviewed By: kbojarczuk Differential Revision: D78560675 fbshipit-source-id: 7602f45c02eecda9ec5bda9b32a9b0d5d23766b7

meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 18, 2025

facebook-github-bot added the fb-exported label Jul 18, 2025

simonmar reviewed Jul 19, 2025

View reviewed changes

pepeiborra pushed a commit to pepeiborra/hsthrift that referenced this pull request Jul 22, 2025

unbreak getdeps build (facebookincubator#160)

96e05c6

Summary: Pull Request resolved: facebookincubator#160 Make getdeps work again Reviewed By: kbojarczuk Differential Revision: D78560675

pepeiborra force-pushed the export-D78560675 branch from 763a073 to 96e05c6 Compare July 22, 2025 09:25

simonmar reviewed Jul 22, 2025

View reviewed changes

pepeiborra pushed a commit to pepeiborra/hsthrift that referenced this pull request Jul 22, 2025

unbreak getdeps build (facebookincubator#160)

bc5f230

Summary: Pull Request resolved: facebookincubator#160 Make getdeps work again Reviewed By: kbojarczuk Differential Revision: D78560675

pepeiborra force-pushed the export-D78560675 branch from 96e05c6 to bc5f230 Compare July 22, 2025 15:28

simonmar reviewed Jul 23, 2025

View reviewed changes

Comment thread cabal-github.project

simonmar Jul 23, 2025

Copy link
Copy Markdown

Contributor

Choose a reason for hiding this comment

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

We should have a default cabal.project, so that the build instructions work.

pepeiborra pushed a commit to pepeiborra/hsthrift that referenced this pull request Jul 23, 2025

unbreak getdeps build (facebookincubator#160)

9245b3f

Summary: Pull Request resolved: facebookincubator#160 Make getdeps work again Reviewed By: kbojarczuk Differential Revision: D78560675

pepeiborra force-pushed the export-D78560675 branch from bc5f230 to 9245b3f Compare July 23, 2025 14:10

unbreak getdeps build (facebookincubator#160)

4275968

Summary: Pull Request resolved: facebookincubator#160 Make getdeps work again Reviewed By: kbojarczuk Differential Revision: D78560675

pepeiborra force-pushed the export-D78560675 branch from 9245b3f to 4275968 Compare July 24, 2025 10:00

facebook-github-bot closed this in 76d157f Jul 25, 2025

facebook-github-bot added the Merged label Jul 25, 2025

		@@ -0,0 +1,23 @@
		-- Copyright (c) Facebook, Inc. and its affiliates.

		packages:

Uh oh!

Conversation

pepeiborra commented Jul 18, 2025

Uh oh!

facebook-github-bot commented Jul 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pepeiborra Jul 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simonmar Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jul 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jul 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jul 23, 2025

Uh oh!

facebook-github-bot commented Jul 24, 2025

Uh oh!

simonmar commented Jul 24, 2025

Uh oh!

facebook-github-bot commented Jul 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pepeiborra Jul 19, 2025 •

edited

Loading

simonmar Jul 22, 2025 •

edited

Loading