Skip to content
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

Remove references of __xstat #330

Merged
merged 1 commit into from
Mar 11, 2021
Merged

Remove references of __xstat #330

merged 1 commit into from
Mar 11, 2021

Conversation

homalozoa
Copy link
Contributor

As mentioned in issue 329.
I've removed some refs of __xstat to fix building errors in Arch Linux. Also tested passed on Ubuntu 18.04 and Ubuntu 20.04.
__xstat was removed in glibc 2.33, so do not use it anymore.

@clalancette
Copy link
Contributor

It looks like the tests failed in CI; that's likely because mimick needs this function to mock everything properly. @hidmic could you take a look and maybe provide some context here?

@homalozoa
Copy link
Contributor Author

homalozoa commented Mar 3, 2021

Maybe I should add a version constraint.

Remove some refs of __xstat to fix building errors in Arch Linux. Also tested passed on Ubuntu 18.04 and Ubuntu 20.04.

__xstat was removed in glibc 2.33, so do not use it anymore.

Signed-off-by: Nxxx <nx.tardis@gmail.com>
@clalancette
Copy link
Contributor

clalancette commented Mar 5, 2021

The idea here looks OK to me. I'm going to run a full CI on it on all platforms to see what happens.

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status
  • Windows Debug Build Status

@clalancette
Copy link
Contributor

All of the unstable tests on Windows are also unstable on the nightlies, so there don't seem to be any regressions here.

I'd still like to get @hidmic's opinion before merging, so I'll hold off on merging this one for now.

@clalancette
Copy link
Contributor

I'm going to go ahead and merge this one. @hidmic feel free to leave comments afterwards if you have any concerns.

@clalancette clalancette merged commit 618a9d9 into ros2:master Mar 11, 2021
@hidmic
Copy link
Contributor

hidmic commented Mar 11, 2021

Argh, this one got lost in my inbox. Patch looks good to me. The thing with (older) glibc is that source API and binary API do not always match (like in this case). That's why these tests are forced to target _xstat in such circumstances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants