Skip to content

[core] Make absl::StrCat import explicit to avoid build failures when using modern absl#48722

Merged
rynewang merged 4 commits intoray-project:masterfrom
eugo-inc:fix-make-absl-strcat-import-explicit
Dec 2, 2024
Merged

[core] Make absl::StrCat import explicit to avoid build failures when using modern absl#48722
rynewang merged 4 commits intoray-project:masterfrom
eugo-inc:fix-make-absl-strcat-import-explicit

Conversation

@gorloffslava
Copy link
Copy Markdown
Contributor

Why are these changes needed?

In src/ray/object_manager/common.cc we extensively use absl::StrCat without explicitely importing the associated header absl/strings/str_cat.h. It worked fine previously, as absl/strings/str_format.h transitively included that via absl internal headers. However, somewhere in 2023-2024, absl has been cleaned up from unintentional transitive includes, breaking ray builds against modern absl. This PR fixes that.

Effect on backward/forward compatability

None. Builds against older absl versions will continue to work as before, as absl::StrCat was always declared in str_cat.h and including that was always the intended way to import this symbol.

Related issues/PRs:

Signed-off-by: gorloffslava <31761951+gorloffslava@users.noreply.github.com>
@gorloffslava gorloffslava changed the title [core] Make absl::StrCat import explicit to avoid build failures in modern absl [core] Make absl::StrCat import explicit to avoid build failures when using modern absl Nov 13, 2024
@pcmoritz
Copy link
Copy Markdown
Contributor

There is a small linting error that needs to be fixed before we can merge this PR :)


b/src/ray/object_manager/common.cc | 0s
-- | --
  | [2024-11-13T09:29:50Z] @@ -15,8 +15,8 @@
  | [2024-11-13T09:29:50Z]  #include "ray/object_manager/common.h"
  | [2024-11-13T09:29:50Z]
  | [2024-11-13T09:29:50Z]  #include "absl/functional/bind_front.h"
  | [2024-11-13T09:29:50Z] -#include "absl/strings/str_format.h"
  | [2024-11-13T09:29:50Z]  #include "absl/strings/str_cat.h"
  | [2024-11-13T09:29:50Z] +#include "absl/strings/str_format.h"
  | [2024-11-13T09:29:50Z]
  | [2024-11-13T09:29:50Z]  namespace ray {
  | [2024-11-13T09:29:50Z]
  | [2024-11-13T09:29:51Z] 🚨 Error: The command exited with status 1


Signed-off-by: Slava Gorlov <31761951+gorloffslava@users.noreply.github.com>
@gorloffslava
Copy link
Copy Markdown
Contributor Author

@pcmoritz fixed + synced with upstream.

@dentiny dentiny added the go add ONLY when ready to merge, run all tests label Nov 15, 2024
@jcotant1 jcotant1 added the core Issues that should be addressed in Ray Core label Nov 15, 2024
@gorloffslava
Copy link
Copy Markdown
Contributor Author

@scv119 gentle bump to see if we can get this merged. Of course, let me know if there’s anything I can help with.

@rynewang rynewang merged commit 5fffbe9 into ray-project:master Dec 2, 2024
jecsand838 pushed a commit to jecsand838/ray that referenced this pull request Dec 4, 2024
… using modern absl (ray-project#48722)

In
[src/ray/object_manager/common.cc](https://github.com/ray-project/ray/blob/e393a716d8742a987a36df555defb2ca90bb94d4/src/ray/object_manager/common.cc)
we extensively use `absl::StrCat` without explicitely importing the
associated header `absl/strings/str_cat.h`. It worked fine previously,
as `absl/strings/str_format.h` transitively included that via absl
internal headers. However, somewhere in 2023-2024, absl has been cleaned
up from unintentional transitive includes, breaking `ray` builds against
modern absl. This PR fixes that.

## Effect on backward/forward compatability
None. Builds against older absl versions will continue to work as
before, as `absl::StrCat` was always declared in `str_cat.h` and
including that was always the intended way to import this symbol.

## Related issues/PRs:
+ ray-project#48667

---------

Signed-off-by: gorloffslava <31761951+gorloffslava@users.noreply.github.com>
Signed-off-by: Slava Gorlov <31761951+gorloffslava@users.noreply.github.com>
Signed-off-by: Connor Sanders <connor@elastiflow.com>
ujjawal-khare pushed a commit to ujjawal-khare-27/ray that referenced this pull request Dec 17, 2024
… using modern absl (ray-project#48722)

In
[src/ray/object_manager/common.cc](https://github.com/ray-project/ray/blob/e393a716d8742a987a36df555defb2ca90bb94d4/src/ray/object_manager/common.cc)
we extensively use `absl::StrCat` without explicitely importing the
associated header `absl/strings/str_cat.h`. It worked fine previously,
as `absl/strings/str_format.h` transitively included that via absl
internal headers. However, somewhere in 2023-2024, absl has been cleaned
up from unintentional transitive includes, breaking `ray` builds against
modern absl. This PR fixes that.

## Effect on backward/forward compatability
None. Builds against older absl versions will continue to work as
before, as `absl::StrCat` was always declared in `str_cat.h` and
including that was always the intended way to import this symbol.

## Related issues/PRs:
+ ray-project#48667

---------

Signed-off-by: gorloffslava <31761951+gorloffslava@users.noreply.github.com>
Signed-off-by: Slava Gorlov <31761951+gorloffslava@users.noreply.github.com>
Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-backlog core Issues that should be addressed in Ray Core go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants