C API for UptaneCycle and campaigns #1263
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| #ifndef AKTUALIZR_LIBAKTUALIZRC_H | ||
| #define AKTUALIZR_LIBAKTUALIZRC_H | ||
|
|
||
| #ifdef __cplusplus | ||
| #include "primary/aktualizr.h" | ||
|
|
||
| using Campaign = campaign::Campaign; | ||
|
|
||
| extern "C" { | ||
| #else | ||
| typedef struct Aktualizr Aktualizr; | ||
| typedef struct Campaign Campaign; | ||
| #endif | ||
|
|
||
| Aktualizr *Aktualizr_create(const char *config_path); | ||
| int Aktualizr_initialize(Aktualizr *a); | ||
| int Aktualizr_uptane_cycle(Aktualizr *a); | ||
| void Aktualizr_destroy(Aktualizr *a); | ||
|
|
||
| Campaign *Aktualizr_campaign_check(Aktualizr *a); | ||
| int Aktualizr_campaign_accept(Aktualizr *a, Campaign *c); | ||
| int Aktualizr_campaign_postpone(Aktualizr *a, Campaign *c); | ||
| int Aktualizr_campaign_decline(Aktualizr *a, Campaign *c); | ||
| void Aktualizr_campaign_free(Campaign *c); | ||
|
|
||
| #ifdef __cplusplus | ||
| } | ||
| #endif | ||
|
|
||
| #endif //AKTUALIZR_LIBAKTUALIZR_H | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| SET(TARGET_NAME aktualizr-c) | ||
| SET(SOURCES libaktualizr-c.cc) | ||
|
|
||
| add_library(${TARGET_NAME} SHARED ${SOURCES}) | ||
| target_include_directories(${TARGET_NAME} PUBLIC ${PROJECT_SOURCE_DIR}/include) | ||
| target_link_libraries(${TARGET_NAME} PRIVATE aktualizr_static_lib ${AKTUALIZR_EXTERNAL_LIBS}) | ||
|
|
||
| aktualizr_source_file_checks(${SOURCES}) | ||
|
|
||
| add_subdirectory(test) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| #include "libaktualizr-c.h" | ||
|
|
||
| Aktualizr *Aktualizr_create(const char *config_path) { | ||
| Aktualizr *a; | ||
| try { | ||
| Config cfg(config_path); | ||
| a = new Aktualizr(cfg); | ||
| } catch (const std::exception &e) { | ||
| std::cerr << "Aktualizr_create exception: " << e.what() << std::endl; | ||
| return nullptr; | ||
| } | ||
| return a; | ||
|
There was a problem hiding this comment. Maybe, there will be a need/requirement to return some status/error code to am app using the C version of libaktualizr, otherwise the only way to find out what went wrong is by parsing stderr. There was a problem hiding this comment. @mike-sul Absolutely. But first, we'll need to make sure that exceptions thrown by libaktualizr are consistent and meaningful. I plan to work on that next. |
||
| } | ||
|
|
||
| int Aktualizr_initialize(Aktualizr *a) { | ||
| try { | ||
| a->Initialize(); | ||
| } catch (const std::exception &e) { | ||
| std::cerr << "Aktualizr_initialize exception: " << e.what() << std::endl; | ||
| return -1; | ||
| } | ||
| return 0; | ||
| } | ||
|
|
||
| int Aktualizr_uptane_cycle(Aktualizr *a) { | ||
| try { | ||
| a->UptaneCycle(); | ||
| } catch (const std::exception &e) { | ||
| std::cerr << "Uptane cycle exception: " << e.what() << std::endl; | ||
| return -1; | ||
| } | ||
| return 0; | ||
| } | ||
|
|
||
| void Aktualizr_destroy(Aktualizr *a) { delete a; } | ||
|
|
||
| Campaign *Aktualizr_campaign_check(Aktualizr *a) { | ||
| try { | ||
| auto r = a->CampaignCheck().get(); | ||
| if (!r.campaigns.empty()) { | ||
| // We don't support multiple campaigns at the moment | ||
| return new Campaign(r.campaigns[0]); | ||
|
There was a problem hiding this comment. Is it fair to assume there will never be multiple campaigns? It would indeed be confusing if there were, but a comment to state the assumption might be worthwhile. There was a problem hiding this comment. It's not supported at the moment, so I think it's worth keeping it simple here. I'll add a comment. |
||
| } | ||
| } catch (const std::exception &e) { | ||
| std::cerr << "Campaign check exception: " << e.what() << std::endl; | ||
| return nullptr; | ||
| } | ||
| return nullptr; | ||
| } | ||
| int Aktualizr_campaign_accept(Aktualizr *a, Campaign *c) { | ||
| try { | ||
| a->CampaignControl(c->id, campaign::Cmd::Accept).get(); | ||
| } catch (const std::exception &e) { | ||
| std::cerr << "Campaign accept exception: " << e.what() << std::endl; | ||
| return -1; | ||
| } | ||
| return 0; | ||
| } | ||
| int Aktualizr_campaign_postpone(Aktualizr *a, Campaign *c) { | ||
| try { | ||
| a->CampaignControl(c->id, campaign::Cmd::Postpone).get(); | ||
| } catch (const std::exception &e) { | ||
| std::cerr << "Campaign postpone exception: " << e.what() << std::endl; | ||
| return -1; | ||
| } | ||
| return 0; | ||
| } | ||
| int Aktualizr_campaign_decline(Aktualizr *a, Campaign *c) { | ||
| try { | ||
| a->CampaignControl(c->id, campaign::Cmd::Decline).get(); | ||
| } catch (const std::exception &e) { | ||
| std::cerr << "Campaign decline exception: " << e.what() << std::endl; | ||
| return -1; | ||
| } | ||
| return 0; | ||
| } | ||
| void Aktualizr_campaign_free(Campaign *c) { delete c; } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| SET(TARGET_NAME api-test) | ||
| SET(SOURCES api-test.c) | ||
|
|
||
| add_executable(${TARGET_NAME} EXCLUDE_FROM_ALL ${SOURCES}) | ||
| add_dependencies(build_tests ${TARGET_NAME}) | ||
| target_link_libraries(${TARGET_NAME} aktualizr-c) | ||
|
|
||
| aktualizr_source_file_checks(${SOURCES}) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| #include <stdio.h> | ||
| #include <stdlib.h> | ||
|
|
||
| #include "libaktualizr-c.h" | ||
|
|
||
| int main(int argc, char **argv) { | ||
| Aktualizr *a; | ||
| Campaign *c; | ||
| int err; | ||
|
|
||
| if (argc != 2) { | ||
| fprintf(stderr, "Missing config file\nUsage:\n\t%s CONFIG_FILE\n", argv[0]); | ||
| return EXIT_FAILURE; | ||
| } | ||
|
|
||
| a = Aktualizr_create(argv[1]); | ||
| if (!a) { | ||
| return EXIT_FAILURE; | ||
| } | ||
|
|
||
| err = Aktualizr_initialize(a); | ||
| if (err) { | ||
| return EXIT_FAILURE; | ||
| } | ||
|
|
||
| c = Aktualizr_campaign_check(a); | ||
| if (c) { | ||
| printf("Accepting running campaign\n"); | ||
| err = Aktualizr_campaign_accept(a, c); | ||
| Aktualizr_campaign_free(c); | ||
| if (err) { | ||
| return EXIT_FAILURE; | ||
|
There was a problem hiding this comment. Will the aktualizr instance be properly tear-downed and freed in this case as well as in case of line #23 ? Or it's not important taking into account that the process is going to die anyway ? There was a problem hiding this comment. It won't, I forgot how to write proper C ;) But I'm rewriting this piece anyway |
||
| } | ||
| } | ||
|
|
||
| err = Aktualizr_uptane_cycle(a); | ||
| if (err) { | ||
| return EXIT_FAILURE; | ||
| } | ||
|
|
||
| Aktualizr_destroy(a); | ||
|
|
||
| return EXIT_SUCCESS; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,9 +17,6 @@ add_aktualizr_test(NAME packagemanagerfake SOURCES packagemanagerfake_test.cc) | |
| # Debian backend | ||
| if(BUILD_DEB) | ||
| set_property(SOURCE packagemanagerfactory.cc packagemanagerfactory_test.cc PROPERTY COMPILE_DEFINITIONS BUILD_DEB) | ||
|
There was a problem hiding this comment. We probably don't need to set this There was a problem hiding this comment. That's a fair point, though we'd ship a code path that calls dpkg on yocto systems where it doesn't make sense. I don't know if the cost of keeping it (low, I think) warrants larger executables and potential bug surface. There was a problem hiding this comment. Probably true. Perhaps we should disable by default on CI and only enable it as necessary? There was a problem hiding this comment. Our approach on CI has been to enable all flags, so that everything can be tested at once. The things that are not covered tend to not compile after refactors. Also, some unrelated useful tests use the Debian mode. For example memory_test. We'll probably migrate them when we can though. There was a problem hiding this comment. Fair enough. I have no objection. There was a problem hiding this comment. @patrickvacek, @lbonn Sorry, I'm a bit confused, what's your conclusion on that? There was a problem hiding this comment. Cool, that's what I thought, just wanted to be sure. There was a problem hiding this comment. Yes, sorry for the distraction! |
||
| target_sources(package_manager PRIVATE debianmanager.cc) | ||
| add_executable(t_packagemanager_deb EXCLUDE_FROM_ALL debianmanager_test.cc) | ||
| add_dependencies(build_tests t_packagemanager_deb) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a bit annoying that we have to pass an Aktualizr instance to all of these. Would be handy if Campaign contained a pointer. But that might need changes in the C++ object, don't know if we want that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see no problem in this, tbh. That's pretty much idiomatic C and is equivalent to
Aktualizr->CampaignAccept(Campaign *c). If we do just a single arg here, I would expect our C++ API to also change toCampaign->Accept(). I'm not sure if it makes it nicer, though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I think you're right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought, maybe it's not a bad thing to make those calls members of Campaign object. This way, we would prevent passing a campaign_id that belongs to another Aktualizr instance (a hypothetical situation, but still). WDYT, @lbonn @patrickvacek @mike-sul ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it doesn't require changes to the C++ code, then that seems like the way to go. Even if it does, that seems to me to be a slightly better way to go.