Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

ref: aktualizr client + secondary interface #1719

Merged
merged 12 commits into from
Jul 27, 2020

Conversation

kbushgit
Copy link
Contributor

@kbushgit kbushgit commented Jul 9, 2020

Signed-off-by: Kostiantyn Bushko kostiantyn.bushko@yahoo.com

Copy link
Collaborator

@pattivacek pattivacek left a comment

Choose a reason for hiding this comment

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

Looks pretty good.

include/libaktualizr/aktualizr.h Show resolved Hide resolved
#include <sodium.h>
#include <chrono>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a big deal, but C++ headers should be listed before external libs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not my fault, make format makes this changed(:

Copy link
Collaborator

Choose a reason for hiding this comment

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

Even if you put empty lines between the C++ and external header sections?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks it works


To use the API, add the `src/libaktualizr` directory to your include path and add `# include "primary/aktualizr.h"` to your source file.
To use the API, add the `aktualizr/include` directory to your include path and add `# include <libaktualizr/aktualizr.h>` to your source file.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not your change, but why is there a space between the # and include?

@kbushgit kbushgit force-pushed the ref/libaktualizr/aktualizr-client branch from 9522dea to 0614de1 Compare July 14, 2020 09:19
@pattivacek
Copy link
Collaborator

I'm not so sure about putting some headers in a secondary subdirectory of the include/libaktualizr directory. Those headers are still specifically for the Primary to use, so to me it seems like a confusing separation. Or was there a specific goal you were trying to achieve with that?

@kbushgit
Copy link
Contributor Author

I'm not so sure about putting some headers in a secondary subdirectory of the include/libaktualizr directory. Those headers are still specifically for the Primary to use, so to me it seems like a confusing separation. Or was there a specific goal you were trying to achieve with that?

No, there is no specific purpose, I'm just not sure if we have to mimic the structure of folders in the public headers as it is in the src?
Is the idea, to put all those headers in one folder more comfortable for you?

@kbushgit kbushgit force-pushed the ref/libaktualizr/aktualizr-client branch from 764c225 to 3a3c924 Compare July 21, 2020 10:35
@codecov-commenter
Copy link

codecov-commenter commented Jul 23, 2020

Codecov Report

Merging #1719 into master will decrease coverage by 3.44%.
The diff coverage is 56.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1719      +/-   ##
==========================================
- Coverage   74.82%   71.37%   -3.45%     
==========================================
  Files         186      186              
  Lines       13723    14381     +658     
==========================================
- Hits        10268    10265       -3     
- Misses       3455     4116     +661     
Impacted Files Coverage Δ
include/libaktualizr/aktualizr.h 100.00% <ø> (ø)
include/libaktualizr/secondary_provider.h 100.00% <ø> (ø)
src/aktualizr_info/main.cc 70.83% <ø> (-22.36%) ⬇️
src/aktualizr_primary/main.cc 65.97% <ø> (ø)
src/aktualizr_primary/secondary_config.cc 63.76% <ø> (-24.24%) ⬇️
src/libaktualizr-isotp/isotpsecondary.cc 0.00% <ø> (ø)
src/libaktualizr-isotp/isotpsecondary.h 0.00% <ø> (ø)
src/libaktualizr-posix/ipuptanesecondary.h 81.81% <ø> (ø)
src/libaktualizr/package_manager/debianmanager.cc 69.04% <0.00%> (-5.32%) ⬇️
src/libaktualizr/package_manager/debianmanager.h 60.00% <ø> (ø)
... and 56 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c861b0...2656eae. Read the comment docs.

Kostiantyn Bushko and others added 12 commits July 23, 2020 17:35
Signed-off-by: Kostiantyn Bushko <kostiantyn.bushko@yahoo.com>
Signed-off-by: Kostiantyn Bushko <kostiantyn.bushko@yahoo.com>
Signed-off-by: Kostiantyn Bushko <kostiantyn.bushko@yahoo.com>
Signed-off-by: Kostiantyn Bushko <kostiantyn.bushko@yahoo.com>
Signed-off-by: Kostiantyn Bushko <kbushko@intellias.com>
Signed-off-by: Kostiantyn Bushko <kbushko@intellias.com>
Signed-off-by: Kostiantyn Bushko <kbushko@intellias.com>
Signed-off-by: Kostiantyn Bushko <kbushko@intellias.com>
Signed-off-by: Kostiantyn Bushko <kbushko@intellias.com>
Signed-off-by: Kostiantyn Bushko <kbushko@intellias.com>
Signed-off-by: Kostiantyn Bushko <kbushko@intellias.com>
Signed-off-by: Kostiantyn Bushko <kbushko@intellias.com>
@kbushgit kbushgit force-pushed the ref/libaktualizr/aktualizr-client branch from 82be3de to 2656eae Compare July 23, 2020 15:42
@@ -37,14 +37,15 @@ data::InstallationResult PartialVerificationSecondary::putMetadata(const Target
}

// TODO(OTA-2484): check for expiration and version downgrade
root_ = Uptane::Root(RepositoryType::Director(), Utils::parseJSON(director_root), root_);
Uptane::Root root(Root::Policy::kAcceptAll);
root = Uptane::Root(RepositoryType::Director(), Utils::parseJSON(director_root), root);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a downgrade in functionality by not storing the Root to check it the next time around. However, this whole class is woefully incomplete, so it doesn't really matter. It needs a lot of work and that isn't part of the scope here.

@pattivacek pattivacek merged commit 7257723 into master Jul 27, 2020
@pattivacek pattivacek deleted the ref/libaktualizr/aktualizr-client branch July 27, 2020 14:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants