Skip to content

Support current icecc codebase#16

Closed
cwyang wants to merge 1 commit intoJPEWdev:masterfrom
cwyang:support-icecream-2025
Closed

Support current icecc codebase#16
cwyang wants to merge 1 commit intoJPEWdev:masterfrom
cwyang:support-icecream-2025

Conversation

@cwyang
Copy link
Copy Markdown

@cwyang cwyang commented Jun 30, 2025

Following refactoring commit of icecc breaks icecream-sundae compilation. This patch fix it:

commit c0f9d537bd47e3326e1126444267cf43bd59696f
Author: Ryan Egesdahl ryan.egesdahl@mongodb.com
Date: Fri Apr 29 20:16:15 2022 -0700

Refactor Msg class

Following refactoring commit of icecc breaks icecream-sundae compilation.
This patch fix it:

commit c0f9d537bd47e3326e1126444267cf43bd59696f
Author: Ryan Egesdahl <ryan.egesdahl@mongodb.com>
Date:   Fri Apr 29 20:16:15 2022 -0700

    Refactor Msg class
@JPEWdev
Copy link
Copy Markdown
Owner

JPEWdev commented Jul 18, 2025

This is unfortunately a breaking API change, so it would break all users with the older version of icecc. Is there a way to detect the version and adjust the code accordingly?

I also added GitHub CI, which should help make sure we aren't breaking things. Thanks

@goatyde
Copy link
Copy Markdown

goatyde commented Feb 25, 2026

One could do the same as done here: icecc/icemon#78

diff --git a/meson.build b/meson.build
index b87a681..0be0909 100644
--- a/meson.build
+++ b/meson.build
@@ -63,6 +63,15 @@
     #add_global_arguments('-D_GLIBCXX_USE_CXX11_ABI=1', language: 'cpp')
 endif
 
+code  = '''
+      #include <icecc/comm.h>
+
+      int main() { Msg msg(M_MON_GET_CS); (void)msg.type; }
+      '''
+if cxx.links(code, dependencies: deps, name: 'old icecc api compatibility')
+    add_global_arguments('-DICECC_TEST_USE_OLD_MSG_API=1', language: 'cpp')
+endif
+
 conf_data = configuration_data()
 conf_data.set('version', meson.project_version())
 configure_file(input: 'config.h.in', output: 'config.h', configuration: conf_data)
diff --git a/src/scheduler.cpp b/src/scheduler.cpp
index 011ca07..5807258 100644
--- a/src/scheduler.cpp
+++ b/src/scheduler.cpp
@@ -28,6 +28,12 @@
 #include "main.hpp"
 #include "scheduler.hpp"
 
+#if ICECC_TEST_USE_OLD_MSG_API
+#define ICECC_MSG_API_COMPAT(old, new) old
+#else
+#define ICECC_MSG_API_COMPAT(old, new) new
+#endif
+
 class IcecreamScheduler: public Scheduler {
 public:
     IcecreamScheduler(std::string const &netname, std::string const &schedname) : Scheduler()
@@ -126,32 +132,32 @@
         return false;
 
     switch (*msg) {
-    case Msg::MON_LOCAL_JOB_BEGIN: {
+    case ICECC_MSG_API_COMPAT(M_LOCAL_JOB_BEGIN, Msg::MON_LOCAL_JOB_BEGIN): {
         auto *m = dynamic_cast<MonLocalJobBeginMsg*>(msg.get());
         Job::createLocal(m->job_id, m->hostid, m->file);
         break;
     }
-    case Msg::JOB_LOCAL_DONE: {
+    case ICECC_MSG_API_COMPAT(M_JOB_LOCAL_DONE, Msg::JOB_LOCAL_DONE): {
         auto *m = dynamic_cast<JobLocalDoneMsg*>(msg.get());
         Job::remove(m->job_id);
         break;
     }
-    case Msg::MON_JOB_BEGIN: {
+    case ICECC_MSG_API_COMPAT(M_JOB_BEGIN, Msg::MON_JOB_BEGIN): {
         auto *m = dynamic_cast<MonJobBeginMsg*>(msg.get());
         Job::createRemote(m->job_id, m->hostid);
         break;
     }
-    case Msg::MON_JOB_DONE: {
+    case ICECC_MSG_API_COMPAT(M_JOB_DONE, Msg::MON_JOB_DONE): {
         auto *m = dynamic_cast<MonJobDoneMsg*>(msg.get());
         Job::remove(m->job_id);
         break;
     }
-    case Msg::MON_GET_CS: {
+    case ICECC_MSG_API_COMPAT(M_GET_CS, Msg::MON_GET_CS): {
         auto *m = dynamic_cast<MonGetCSMsg*>(msg.get());
         Job::createPending(m->job_id, m->clientid, m->filename);
         break;
     }
-    case Msg::MON_STATS: {
+    case ICECC_MSG_API_COMPAT(M_STATS, Msg::MON_STATS): {
         auto *m = dynamic_cast<MonStatsMsg*>(msg.get());
         auto host = Host::create(m->hostid);

@JPEWdev
Copy link
Copy Markdown
Owner

JPEWdev commented Feb 25, 2026

Seems fine. FWIW, I'm not using icecc anymore, so I'm not likely to fix this myself. If you want to submit a PR, that's fine. If there would be a better owner of this project, I'd also be willing to transfer ownership

@goatyde
Copy link
Copy Markdown

goatyde commented Feb 26, 2026

I´m just using icecc as bazel is too complicated. I´ll try and do a pull request. Too bad there´s such a lack of maintainers for FOSS.

@goatyde goatyde mentioned this pull request Feb 26, 2026
@JPEWdev
Copy link
Copy Markdown
Owner

JPEWdev commented Feb 26, 2026

I´m just using icecc as bazel is too complicated. I´ll try and do a pull request. Too bad there´s such a lack of maintainers for FOSS.

Ya, time can be at a premium. The real problem with this tool is that you need a working icecc cluster to test it, which i don't really have access to anymore. I'm more than happy to take PRs and review them, but I won't notice things like this preemptively since I'm not using it every day like I used to.

@JPEWdev
Copy link
Copy Markdown
Owner

JPEWdev commented Feb 28, 2026

Fixed in #18

@JPEWdev JPEWdev closed this Feb 28, 2026
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