Borg: Update to v1.4.1 and migrate to Python 3.12 #6444
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7 tasks
This comment was marked as outdated.
This comment was marked as outdated.
|
Hey @th0ma7, I’d appreciate your thoughts on the comments in the Makefile: spksrc/spk/borgbackup/Makefile Lines 12 to 22 in f470d40 The current approach seems quite dependent on an existing build environment and might reinforce what's already in the repo instead of building directly from source. I’d like to suggest the following revision, which aligns with how I approached this PR: |
|
@mreid-tt you can create the requirements files with native python only in the spksrc environment. I use a script The output of this script is three files
When using native/python* only, you have to install additional dependencies into the dev environment (most python packages do not have such dependencies) for borgbackup (with llfuse) install once (before running the script): running for borg package using native/python312: creates the following files: generated_requirements_all.txt generate_requirements.sh is available as gist https://gist.github.com/hgy59/6676a55ea8b0b0264af7f23850f5e7c8 I use this script mainly for Homeassistant, which comes with a tree structure of requirement files for the many wheels. |
|
@hgy59, thanks for sharing your script again and including the prerequisites. I realized I had forgotten to mention installing those as well, which I needed to do in my setup. Your script is definitely easier to run, but as I mentioned earlier, I don’t have a local environment due to running on an ARM-based architecture, which isn’t compatible with Interestingly, the output from my method matches the output of your script exactly, so I’m not entirely sure why you mentioned this can only be done in the Build LogI’d appreciate hearing your thoughts on this. |
The script uses python from ../../../native/python* (and it runs make for native python on demand), this is why it runs in spksrc context only. Some wheels have python version dependent dependencies. With the native python variant it is easy to have parallel 311, 312 and 313 installations available. |
|
Sort of thinking out loud here.... in theory one could create a crossenv definition file specific for instance to borgbackup. Then when testing your build you comment out all your wheels and manually invoke Maybe one could even pass along the REQUIREMENT="file" as well (I'd need to revisit the code to confirm). This way we could create a requirement definition file for each of our python apps, located within its own directory and simply update that as needed, testing it from crossenv, adjusting our final src/requirements files to match the versions and voila! |
interesting thoughts... |
|
Indeed faster, but testing from crossenv/cross would be a direct replication of the end result. Although this could also be true from crossenv/build where pip would use pre-built wheels, thus as fast as what you're using. |
|
Hey @hgy59, I was able to use your script method by setting up a basic local environment in my VM, and it worked perfectly. This seems like a consistent approach for updating requirements in Python builds. However, I’ll still need to use GitHub for other builds, as the toolchains don't support ARM architectures. I've attached the build log below for your reference. Build LogBased on this success, the Makefile instructions could potentially be updated as follows: Let me know if you have any thoughts on this. |
mreid-tt
force-pushed
the
borgbackup-update
branch
3 times, most recently
from
29d75e6 to
30672c5
Compare
mreid-tt
force-pushed
the
borgbackup-update
branch
from
30672c5 to
6411eb1
Compare
|
@mreid-tt can you PLEASE avoid force-pushing of your commits? If you force-push a branch, you rewirte the history and this breaks the co-development of any other user. @th0ma7 recently merged #6454 with 82 commits and non of it was force-pushed. Rule of thumb
|
|
@hgy59 Thanks for the reminder about best practices. I was trying to rebase to include the latest changes from master. The last time I merged master, it resulted in a bunch of individual commits instead of the single merge commit I usually see you do. I just tried it again with the transmission-beta (see PR #6415), and it worked as expected this time. I’m not sure what I did differently before, but now that I’ve confirmed it’s working, I’ll make sure to follow the guidelines you outlined. |
|
@mreid-tt and @hgy59 how I normally do things is (and let me know if you have a better method): |
My normal usecase (I use tortoise-git on windows, but try to list git cli commands) With this, most of the time, no rebase is required. |
|
I'll have to try that out. And indeed I also often use github page to updat my fork... varies. |
6 tasks
|
FYI, borgbackup was just updated to version 1.4.1 |
mreid-tt
changed the title
|
Install and update seem to be successful on DSM 6: Install LogUpgrade LogA few test commands also seem to be successful: |
mreid-tt
added
status/published
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR includes the following:
Fixes #
Checklist
all-supportedcompleted successfullyType of change