Skip to content

system/nxpkg: Complete the package lifecycle and command line. - #3720

Open
aviralgarg05 wants to merge 2 commits into
apache:masterfrom
aviralgarg05:gsoc/nxpkg-lifecycle
Open

system/nxpkg: Complete the package lifecycle and command line.#3720
aviralgarg05 wants to merge 2 commits into
apache:masterfrom
aviralgarg05:gsoc/nxpkg-lifecycle

Conversation

@aviralgarg05

Copy link
Copy Markdown
Contributor

Note: Please adhere to Contributing Guidelines.

Depends-On: /pull/3719

Summary

Install wrote the current and previous pointer files, and removed the
payload of a pruned version, around the database save rather than after it.
A failure in between left the database naming a version whose files were
already gone. Reinstalling a version that was already present treated its
directory as newly created, so a failed reinstall deleted a working
install. Uninstall and rollback took only the database lock, which left
another install free to work on the same package at the same time.

The database is now committed first, and only then are pointers refreshed
or files deleted. A crash can therefore strand files that are reclaimable,
but never leaves the database pointing at a payload that is gone. Uninstall
and rollback take the per-package lock as well, and install picks up state
left behind by an earlier attempt that was interrupted.

Only install, list and available were reachable from the shell, so a
package could be put on a device but never updated, rolled back or removed.
sync, update, remove and rollback are now wired up, and the CLI
reports the errno the library returns rather than a plain failure.

This is the last of four parts of #3642, which was one commit covering
several unrelated changes.

Impact

  • New feature: YES, update, rollback and remove become usable.
  • User adaptation: NO.
  • Build: NO.
  • Hardware: NO.
  • Documentation: The companion documentation is Documentation: Update nxpkg and add nxstore guide. nuttx#18875.
  • Security: NO new boundary. The ordering change removes a way to lose a
    payload the database still refers to.
  • Compatibility: An existing installed database is read as before.

Testing

Build host: macOS 26.5, arm64, xtensa-esp-elf-gcc 14.2.0
(esp-14.2.0_20251107).

Target: Xtensa / ESP32-S3, Waveshare ESP32-S3-Touch-LCD-7.

On the target, against a repository on the SD card, the following were
exercised: install, update to a second version, rollback to the first,
remove, a package lock held against a second install returning -EBUSY, a
lock reclaimed from an exited owner and from an earlier boot, and an
artifact whose SHA-256 did not match, which left no database entry, version
directory, lock or transaction file behind.

This commit does not build on master alone because it uses declarations
and helpers added by the three parts before it; the Depends-On line above
lets CI apply those first.

PR verification Self-Check

  • This PR introduces one focused change.
  • I have updated all required description fields above.
  • I have reviewed and signed every commit.
  • This PR adheres to the current contribution and coding guidelines.
  • My PR is still work in progress.
  • My PR is ready for review and can be safely merged.

…disk.

Install wrote the pointer files and removed payloads around the database
save rather than after it, so a failure in between could leave the database
naming a version whose files were already gone. Reinstalling a version that
was already present treated its directory as newly created, so a failed
reinstall deleted a working install. Uninstall and rollback took only the
database lock, leaving another install free to work on the same package.

Commit the database first and only then refresh the pointers or delete
files, so a crash can strand files that are reclaimable but never lose a
payload the database still points at. Take the per-package lock in uninstall
and rollback as well, and pick up state left by an install that was
interrupted earlier.

Assisted-by: OpenAI Codex:gpt-5.6-sol
Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
Only install, list and available were reachable from the shell, so a
package could be put on a device but never updated, rolled back or taken
off again.

Wire up sync, update, remove and rollback, and report the errno the library
returns rather than a plain failure.

Assisted-by: OpenAI Codex:gpt-5.6-sol
Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
@github-actions

Copy link
Copy Markdown

🔗 Cross-repo PR dependencies

The read-only Build run reported the following dependent PR(s) and fetched head SHA(s):

CI run: https://github.com/apache/nuttx-apps/actions/runs/31526679208

for (i = victim; i + 1 < entry->version_count; i++)
{
memcpy(entry->versions[i], entry->versions[i + 1],
sizeof(entry->versions[i]));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add space

pkg_free(payload);
pkg_free(manifest_path);
pkg_free(lock);
pkg_free(installed_lock);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could we goto the end to avoid the code dup

pkg_free(lock);
pkg_free(installed_lock);
pkg_error("unable to allocate package metadata buffers");
return EXIT_FAILURE;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

goto errout to avoid the code dup

pkg_free(payload);
pkg_free(manifest_path);
pkg_free(lock);
pkg_free(installed_lock);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ditto

pkg_free(payload);
pkg_free(manifest_path);
pkg_free(lock);
pkg_free(installed_lock);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ditto

pkg_free(manifest_path);
pkg_free(lock);
pkg_free(installed_lock);
pkg_error("unable to resolve artifact source for '%s': %d", name, ret);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ditto

pkg_free(payload);
pkg_free(manifest_path);
pkg_free(lock);
pkg_free(installed_lock);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ditto

{
pkg_store_remove_file(installed_lock);
pkg_store_remove_file(package_lock);
pkg_free(db);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could we use goto to avoid the code duplication too

{
pkg_store_remove_file(installed_lock);
pkg_store_remove_file(package_lock);
pkg_free(db);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ditto

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants