Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cross/libsodium: update to v1.0.20 #6449

Merged
merged 5 commits into from
Feb 17, 2025

Conversation

mreid-tt
Copy link
Contributor

@mreid-tt mreid-tt commented Feb 12, 2025

Description

This PR includes the following changes:

  1. Update to libsodium v1.0.20

Fixes #

Checklist

  • Build rule all-supported completed successfully
  • New installation of package completed successfully
  • Package upgrade completed successfully (Manually install the package again)
  • Package functionality was tested
  • Any needed documentation is updated/created

Type of change

  • Library update

@mreid-tt mreid-tt self-assigned this Feb 12, 2025
@mreid-tt mreid-tt marked this pull request as draft February 12, 2025 21:10
@hgy59
Copy link
Contributor

hgy59 commented Feb 13, 2025

@mreid-tt you forgot to update the PLIST file

@mreid-tt
Copy link
Contributor Author

@hgy59, I added this yesterday because the source repo was down, and I noticed they also had a GitHub source. However, I later realized that the GitHub source doesn’t always include a "stable" version for all releases, unlike the original repo. Now that the source repo is back up, I’m considering canceling this — what do you think?

@hgy59
Copy link
Contributor

hgy59 commented Feb 13, 2025

the GitHub source doesn’t always include a "stable" version for all releases

If the original source repo has the update too, I recommend to proceed with this PR. If you don't know the exact version for PLIST (I know, you are lacking a dev env), I can evaluate those for you.

had the same finding for cross/libxml2 so switched back to original sources (while downgrading in #6440).

@mreid-tt
Copy link
Contributor Author

If the original source repo has the update too, I recommend to proceed with this PR. If you don't know the exact version for PLIST (I know, you are lacking a dev env), I can evaluate those for you.

I've updated the changes based on the original source, incorporating their new naming convention with an explicit "stable" in the filename. If you could assist with updating the PLIST, I'd appreciate it.

@hgy59
Copy link
Contributor

hgy59 commented Feb 13, 2025

I've updated the changes based on the original source, incorporating their new naming convention with an explicit "stable" in the filename. If you could assist with updating the PLIST, I'd appreciate it.

Just curious why you didn't take the stable release anymore.

there is a release 1.20.0 from 25-May-2024 12:22
and a release 1.20.0-stable from 16-Jan-2025 13:42

@mreid-tt
Copy link
Contributor Author

Just curious why you didn't take the stable release anymore.

I thought I was. When you extract libsodium-1.0.19.tar.gz you get a folder libsodium-stable but when you extract libsodium-1.0.20.tar.gz you get a folder libsodium-1.0.20. However when you extract libsodium-1.0.20-stable.tar.gz you get a folder libsodium-stable. As such, I thought they changed their naming. But if the file without the name "stable" in the title is in fact the stable version then you would need to update the PKG_DIR.

- use commit date as PKG_VERS to create unique download files for digests
- use PLIST to document kind of installed files
@mreid-tt
Copy link
Contributor Author

mreid-tt commented Feb 14, 2025

@hgy59, I think I got a better understanding of this from jedisct1/libsodium#1373 (comment):

Recently, a new cycle started with the RELEASE-1.0.20 tag. stable now represents version 1.0.20 with minor fixes and improvements (but no breaking changes nor new features). And since a new cycle started, tag 1.0.19 represents the last stable 1.0.19 version, identical to the last 1.0.19-stable tarball. If your application requires 1.0.19 and can't use 1.0.20, you can use this instead of 1.0.19-RELEASE. It will, for example, ensure that it can be compiled with recent compiler versions while 1.0.19 as originally release can't.

You can stick to the -RELEASE tags if you want. Or use stable to always get the latest release with backported fixes and improvements before the next point release.

It looks like I had it backward. The filename without "stable" refers to the first release of that version, while the one with "stable" represents the latest version within that point release. If that’s the case, our digests will periodically fail as the upstream team updates their fixes. How do you feel about using the latest version moving forward, or should we stick with the initial release for better stability in our build system?

@hgy59
Copy link
Contributor

hgy59 commented Feb 14, 2025

It looks like I had it backward. The filename without "stable" refers to the first release of that version, while the one with "stable" represents the latest version within that point release. If that’s the case, our digests will periodically fail as the upstream team updates their fixes. How do you feel about using the latest version moving forward, or should we stick with the initial release for better stability in our build system?

@mreid-tt you are right (and the comment in the referenced issue is absolutly corret: The moving tags (used like a branch) are quite confusing, and difficult to carry through different trackers.)

It is very bad practice to release *-stable versions that get continues updates without changing the version number (If that is the case here).

So we go better back to the real stable version (i.e. the one that does not change) the one without stable in the name.

@mreid-tt sorry for not digging into the details yesterday.

This reverts commit 9a5a22d.

revert to the invariant version 1.0.20
Copy link
Contributor

@hgy59 hgy59 left a comment

Choose a reason for hiding this comment

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

LGTM

@mreid-tt mreid-tt marked this pull request as ready for review February 15, 2025 01:23
@mreid-tt
Copy link
Contributor Author

@hgy59, thanks for the approval. Before merging, I wanted to check — since we're sticking with the initial release on this repo, should I re-implement the source switch to GitHub? The first release is the identical file, and using GitHub as the source might help reduce potential build failures. Let me know your thoughts.

@mreid-tt mreid-tt merged commit e0927da into SynoCommunity:master Feb 17, 2025
14 of 15 checks passed
@mreid-tt mreid-tt deleted the libsodium-update branch February 17, 2025 20:59
@hgy59
Copy link
Contributor

hgy59 commented Feb 17, 2025

@mreid-tt I have cancelled the workflow since it already succeeded on the branch (and we have a known issue since #6437)

To answer your latest question:
we could have switched back to github repo, but we do not know, how future releases are provided. So far we just should avoid the so called "stable" versions of libsodium...

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.

2 participants