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

Add seastar 2211 version to infra #1134

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

travisdowns
Copy link
Contributor

The existing seastar version 1808 is more than five years old and lots of exciting changes have occurred since then. This patch adds seastar 2211 from last last year so that it can be be exposed by CE.

The existing seastar version 1808 is more than five years old and
lots of exciting changes have occurred since then. This patch
adds seastar 2211 from last last year so that it can be be exposed
by CE.
travisdowns added a commit to travisdowns/compiler-explorer that referenced this pull request Oct 29, 2023
The existing seastar version 1808 is more than five years old and
lots of exciting changes have occurred since then. This patch
adds seastar 2211 from last last year as well as trunk.

The 2211 version of seastar requires PR compiler-explorer/infra#1134
to be accepted first to add it on the infra side.
Copy link
Member

@mattgodbolt mattgodbolt left a comment

Choose a reason for hiding this comment

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

Thanks Travis!

@mattgodbolt mattgodbolt merged commit ea5e373 into compiler-explorer:main Oct 31, 2023
@mattgodbolt
Copy link
Member

There's no trunk install, but I'll hack that now.

@mattgodbolt
Copy link
Member

Oh...there is...sorry! 😊

mattgodbolt pushed a commit to compiler-explorer/compiler-explorer that referenced this pull request Oct 31, 2023
The existing seastar version 1808 is more than five years old and lots
of exciting changes have occurred since then. This patch adds seastar
2211 from last last year as well as trunk.

The 2211 version of seastar requires PR compiler-explorer/infra#1134 to
be accepted first to add it on the infra side.

`make check` passes.
@travisdowns
Copy link
Contributor Author

Thanks so much for merging this @mattgodbolt - we are having a lot of fun with it already!

A couple of questions came up while we making this PR, I guess either you or @partouf would know:

  • Is there a way to declare that the use of on library in CE requires another? E.g., sesatar needs fmt, so that when you select fmt in the UI, fmt would also be enabled (guessing no, but thought I'd check). I see a "dependencies" concept but I think it's for link time?
  • Seastar headers requires a default define to compile at all, like -DSEASTAR_SCHEDULING_GROUPS=16. I just add it to the compiler command line manually, but is there any way to specify this in the CE properties so it happens automatically?

@partouf
Copy link
Member

partouf commented Nov 2, 2023

Dependencies are mostly for linking yes, dependencies are hard because you would have to pre-define the version of the library, where we'd like to keep some flexibility.

There are however some libraries that add the include paths of specific versions to their own, so it's automatic, but also hardcoded. (example https://github.com/compiler-explorer/compiler-explorer/blob/4a8fdbf81005517e9b77242856d2651d20438145/etc/config/c%2B%2B.amazon.properties#L3822)

What we usually do is just add something to the description of the library (example https://github.com/compiler-explorer/compiler-explorer/blob/main/etc/config/c%2B%2B.amazon.properties#L3186)

Regarding the define: QT needed that too, we solved it with this https://github.com/compiler-explorer/compiler-explorer/blob/4a8fdbf81005517e9b77242856d2651d20438145/etc/config/c%2B%2B.amazon.properties#L3979 - note that you will not be able to override this define.

@Genomorf
Copy link

Thank you for adding the new seastar version to CE. Happy to see it.
@travisdowns could you please help me to figure out how to use it? I can't run simple Hello World application in CE. I guess i miss some crucial command line arguments. Link: https://godbolt.org/z/nx1K5E7b1

My setup is clang 16.0, args are: -std=c++20 -O3 -DSEASTAR_SCHEDULING_GROUPS_COUNT=16. It compiles but i get linker error: undefined reference to `seastar::app_template::app_template(seastar::app_template::config)'. Looks like CE is unable to find definition of the function. How to fix it? Do i need to pass some args to a linker?

@travisdowns
Copy link
Contributor Author

@Genomorf here's a working link:

https://gcc.godbolt.org/z/WjG3aoY64

Keys are:

Define SEASTAR_SCHEDULING_GROUPS_COUNT either before header include or with -D on the command line.
Include boost and fmt libraries. Currently trunk version works for those.
Use -std=c++17 or 20, or for trunk, 23.

@Genomorf
Copy link

@travisdowns thank you for the answer, but your example has the same linker error. If you open the "output" tab you can see undefined reference to `seastar::yield()' error.

I use CE as a sandbox platform to run the code, print messages, experiment with the tasks. I can't do it with your example. It only compiles.

Is there an option to actually run the code?

@partouf
Copy link
Member

partouf commented Sep 21, 2024

We do not build binaries for seastar, so linking will not work.

We can look into it

@partouf
Copy link
Member

partouf commented Sep 21, 2024

#1415

@Genomorf
Copy link

@partouf is it hard for CE to build binaries for seastar? I can help and try to solve the issue if there is a guide or example, or at least some directions how to do it.

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.

4 participants