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
folly: update to 2022.08.08.00, add fixes for gcc and older OS #15689
Conversation
P. S. I wanted to make sure updated port builds fine on new OSs, but please give me some time to fix PPC build. |
As of now, three problems have been discovered, as long as PPC goes:
I do not have a solution for the latter one. @mascguy @ryandesign @kencu Any ideas in this regard? |
Then, there are problems with Details
UPD. This is not specific to PPC, but to OS < 10.8. Can be fixed like this: https://svn.ruby-lang.org/cgi-bin/viewvc.cgi/trunk/ext/-test-/memory_status/memory_status.c?view=markup&pathrev=57180 |
For the
So in short, don't define the constant. (Because if it's not defined already, it means the platform doesn't support it.) Instead, check whether it's defined, and make the appropriate call based on that. I'm not an expert on this call though, so I'll defer to Ken and Ryan for guidance. |
I appreciate your work on this, since Facebook libraries are so painful to maintain. |
@mascguy Appreciated! I will update the patch. |
After fixing the about, build errs out on: Details
|
Atomics can be fixed, other symbols need to check. Wonder why |
UPD. So yeah, atomics fixed, this remains: Details
|
So the only problem remaining is failure to link with This failure occurs at the very end of the build. If we can fix it, perhaps we have I could try adding <gflags/gflags.h> to problematic files manually and change Cmake setting for Gflags namespace. Other than that, I dunno what to try. |
@catap So the symbols differ in DetailsCompare this:
To this:
|
@catap This may be relevant: https://kumasento.github.io/2020-06-12-glog-gflags-and-c-abi/ |
@mascguy @aeiouaeiouaeiouaeiouaeiouaeiou Magic!
We will need a minor addition to this PR: #15678 UPD. All done. I hope it fixes Intel builds too, though this has to be tested on buildbots. |
P. S. Making this work on PPC also requires a fix for Boost, so that |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
@mascguy Are we ready to try this out? Once/if everything builds fine across the board, I can finalize dependents (currently |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, let's merge.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
@mascguy Thank you! I see that Mavericks and Yosemite now build (they did not before, as I recall). Looks like we have fixed at least some systems :) For For 10.7 and 10.8: I cannot really fix something libcxx-related (at least right now), but the question is: is dependency on it really required? (Again, I do not know that.) GCC builds the port fine without libcxx. |
The recent update to folly broke building watchman. However, a simple upgrade doesn't work, as they added yet another dependency to something; their Eden SCM. Eden isn't in MacPorts, and I _really_ don't care about that one, so I disabled it -- which allowed pruning the list of dependencies a bit. But they moved some code to rely on it anyway, so I reverted those commits. Since the update to Folly broke this, I added a comment to the port.
@mascguy @kencu The issue with |
Yep, sure enough, they needed to add the include. |
This comment was marked as outdated.
This comment was marked as outdated.
@mascguy Do you mean this?
This excludes only For TCP fast-open, this was already in
|
Ah, missed that block. We're good then! |
Ah, gotcha. We might need to patch the source file in question, adding a minimum macOS version surrounding the identifier. Not sure if it's that easy, but we can test next week. |
There seems to be some issue either with gcc12 or new
I will find out where things go wrong. |
Description
This PR:
Type(s)
Tested on
macOS 10.6.8 Server
Xcode 3.2.6
Verification
Have you
port lint --nitpick
?sudo port test
?sudo port -vst install
?