PengZheng commented on code in PR #476:
URL: https://github.com/apache/celix/pull/476#discussion_r1142216707


##########
libs/framework/src/bundle_archive.c:
##########
@@ -28,644 +28,451 @@
 #include <dirent.h>
 #include <unistd.h>
 #include <string.h>
+#include <assert.h>
 
 #include "celix_constants.h"
 #include "celix_utils_api.h"
 #include "linked_list_iterator.h"
 #include "framework_private.h"
 #include "celix_file_utils.h"
 #include "bundle_revision_private.h"
+#include "celix_framework_utils_private.h"
 
+
+celix_status_t celix_bundleArchive_getLastModifiedInternal(bundle_archive_pt 
archive, struct timespec *lastModified);
+
+/**
+ * The bundle archive which is used to store the bundle data and can be reused 
when a framework is restarted.
+ * The lifecycle of a bundle archive is coupled to the lifecycle of the bundle 
that is created from the archive.
+ *
+ * @note The bundle archive is thread safe.
+ */
 struct bundleArchive {
+    //initialed during creation and immutable
        celix_framework_t* fw;
        long id;
-       char * location;
-       DIR *archiveRootDir;
-       char * archiveRoot;
-       linked_list_pt revisions;
-       long refreshCount;
-       time_t lastModified;
-
-       bundle_state_e persistentState;
-};
+       char *archiveRoot;
+    char *savedBundleStatePropertiesPath;
+    char* storeRoot;
+    bool isSystemBundle;
+    char* bundleSymbolicName; //read from the manifest
+    char* bundleVersion; //read from the manifest
 
-static celix_status_t bundleArchive_getRevisionLocation(bundle_archive_pt 
archive, long revNr, char **revision_location);
-static celix_status_t bundleArchive_setRevisionLocation(bundle_archive_pt 
archive, const char * location, long revNr);
 
-static celix_status_t bundleArchive_initialize(bundle_archive_pt archive);
+    celix_thread_mutex_t lock; //protects below and saving of bundle state 
properties
 
-static celix_status_t 
bundleArchive_createRevisionFromLocation(bundle_archive_pt archive, const char 
*location, const char *inputFile, long revNr, bundle_revision_pt 
*bundle_revision);
-static celix_status_t bundleArchive_reviseInternal(bundle_archive_pt archive, 
bool isReload, long revNr, const char * location, const char *inputFile);
+    char* location;
+    char* currentRevisionRoot;
+    celix_array_list_t* revisions; //list of bundle_revision_t*
 
-static celix_status_t bundleArchive_readLastModified(bundle_archive_pt 
archive, time_t *time);
-static celix_status_t bundleArchive_writeLastModified(bundle_archive_pt 
archive);
+    //bundle cache state info
+       long revisionNr;
 
-celix_status_t bundleArchive_createSystemBundleArchive(celix_framework_t* fw, 
bundle_archive_pt *bundle_archive) {
-       celix_status_t status = CELIX_SUCCESS;
-       char *error = NULL;
-       bundle_archive_pt archive = NULL;
-
-       if (*bundle_archive != NULL) {
-               status = CELIX_ILLEGAL_ARGUMENT;
-               error = "Missing required arguments and/or incorrect values";
-       } else {
-               archive = (bundle_archive_pt) calloc(1,sizeof(*archive));
-               if (archive == NULL) {
-                       status = CELIX_ENOMEM;
-               } else {
-                       status = linkedList_create(&archive->revisions);
-                       if (status == CELIX_SUCCESS) {
-                               archive->fw = fw;
-                               archive->id = CELIX_FRAMEWORK_BUNDLE_ID;
-                               archive->location = strdup("System Bundle");
-                               archive->archiveRoot = NULL;
-                               archive->archiveRootDir = NULL;
-                               archive->refreshCount = -1;
-                               archive->persistentState = 
CELIX_BUNDLE_STATE_UNKNOWN;
-                               time(&archive->lastModified);
-
-                               *bundle_archive = archive;
-                       }
-               }
-       }
-
-       if(status != CELIX_SUCCESS && archive != NULL){
-               bundleArchive_destroy(archive);
-       }
-
-       framework_logIfError(fw->logger, status, error, "Could not create 
archive");
+    //bundle cache state in properties form
+    celix_properties_t* bundleStateProperties;
+};
 
-       return status;
+static void 
celix_bundleArchive_updateAndStoreBundleStateProperties(bundle_archive_pt 
archive) {
+    celixThreadMutex_lock(&archive->lock);
+    //set/update bundle cache state properties
+    celix_properties_setLong(archive->bundleStateProperties, 
CELIX_BUNDLE_ARCHIVE_BUNDLE_ID_PROPERTY_NAME, archive->id);
+    celix_properties_set(archive->bundleStateProperties, 
CELIX_BUNDLE_ARCHIVE_LOCATION_PROPERTY_NAME, archive->location);
+    celix_properties_set(archive->bundleStateProperties, 
CELIX_BUNDLE_ARCHIVE_SYMBOLIC_NAME_PROPERTY_NAME, archive->bundleSymbolicName);
+    celix_properties_set(archive->bundleStateProperties, 
CELIX_BUNDLE_ARCHIVE_VERSION_PROPERTY_NAME, archive->bundleVersion);
+    celix_properties_setLong(archive->bundleStateProperties, 
CELIX_BUNDLE_ARCHIVE_REVISION_PROPERTY_NAME, archive->revisionNr);

Review Comment:
   Though `archive->revisionNr` is stored, it seems never reloaded after 
framework restart? 
   `celix_bundleArchive_createArchiveInternal` always receives 1.
   Is is intentional or a bug? If a bug, then `update` command is also broken, 
which is worth of a separate issue.
   
   Please correct me if I were wrong.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to