Skip to content

Conversation

Aidyn-A
Copy link
Contributor

@Aidyn-A Aidyn-A commented Nov 20, 2024

The TOT compilation fails with:

FAILED: third_party/gloo/gloo/CMakeFiles/gloo.dir/transport/tcp/buffer.cc.o 
...
~/git/pytorch/third_party/gloo/gloo/transport/tcp/buffer.cc: In member function ‘virtual void gloo::transport::tcp::Buffer::send(size_t, size_t, size_t)’:
~/git/pytorch/third_party/gloo/gloo/transport/tcp/buffer.cc:130:53: error: ‘__NR_gettid’ was not declared in this scope
  130 |     std::cout << "[" << getpid() << ": " << syscall(__NR_gettid) << "] ";

due to missing sys/syscall.h header declaration in buffer.cc introduced in commit aeca183.

@Aidyn-A
Copy link
Contributor Author

Aidyn-A commented Nov 21, 2024

cc @c-p-i-o

@c-p-i-o c-p-i-o self-requested a review November 26, 2024 21:33
@c-p-i-o
Copy link
Contributor

c-p-i-o commented Nov 26, 2024

oops sorry. I really need to go look at fixing the internal codemod here at facebook. Because internally we use a bazel like build, it seems to ignore build variations.

@c-p-i-o
Copy link
Contributor

c-p-i-o commented Nov 26, 2024

Also, I'm trying to revive the CI via github workflows.
Feel free to add any automation there - once I land #395, and #396

@c-p-i-o
Copy link
Contributor

c-p-i-o commented Nov 27, 2024

Small CI has already landed.
However, I discovered that it's hard for someone in Open Source to land changes into the main branch.
Let me figure out how to land changes for you.

@facebook-github-bot
Copy link

@c-p-i-o has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@c-p-i-o
Copy link
Contributor

c-p-i-o commented Dec 2, 2024

Interestingly, the Facebook internal linter doesn't like this change.

Screenshot 2024-12-02 at 2 40 51 PM

@c-p-i-o
Copy link
Contributor

c-p-i-o commented Dec 2, 2024

Trying with#include <asm/unistd_64.h> instead.

@c-p-i-o
Copy link
Contributor

c-p-i-o commented Dec 3, 2024

Seems like the internal linter is happy is I use:

#include <asm/unistd_64.h>

Instead of what you have.

Does using the header above resolve the linker error for you @Aidyn-A ?

@Aidyn-A
Copy link
Contributor Author

Aidyn-A commented Dec 3, 2024

I think it will no longer be system agnostic once we do #include <asm/unistd_64.h>. Calling syscall(SYS_gettid) instead of syscall(__NR_gettid) should make clang-tidy happy.

@facebook-github-bot
Copy link

@c-p-i-o has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot merged commit dc507d1 into pytorch:main Dec 3, 2024
5 of 6 checks passed
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.

3 participants