ENT-12414: Use librsync's Stream API in GET <FILENAME> requests
#5629
Conversation
This file contains 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
larsewi
force-pushed
the
librsync
branch
7 times, most recently
from
c29dfd8 to
937589a
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
larsewi
force-pushed
the
librsync
branch
8 times, most recently
from
37a8af2 to
9cb3c14
Compare
larsewi
force-pushed
the
librsync
branch
3 times, most recently
from
fafd971 to
af5bd02
Compare
vpodzime
reviewed
Nov 29, 2024
larsewi
force-pushed
the
librsync
branch
3 times, most recently
from
b627503 to
2e808ed
Compare
vpodzime
previously approved these changes
Dec 2, 2024
libcfnet/file_stream.c
Outdated
| /* librsync has rs_file_size() which basically does the same thing. | ||
| * However, it is only available in the most recent versions of the | ||
| * library. Hence, we provide our own implementation in order to have | ||
| * some backwards compatibility in terms of librsync versions. */ |
There was a problem hiding this comment.
Can you be specific here about the version in which rs_file_size() was added?
Are there any minimum version notes in INSTALL and configure.ac that prevent problems with the code as-is?
There was a problem hiding this comment.
Added some more info in the comment. When it comes to minimum version checks, it does not seem like we have this for any of the other dependencies. If you think we should have these checks, then we should probably make a separate ticket for this.
larsewi
force-pushed
the
librsync
branch
5 times, most recently
from
f728290 to
8ad0742
Compare
craigcomstock
previously approved these changes
Dec 6, 2024
|
Just tested this manually on Windows and it works like a charm. Did however find a separate (surprisingly unrelated) issue in |
olehermanse
reviewed
Dec 11, 2024
This commit was added while working on implementing the File Stream API in ticket ENT-12414. The File Stream API utilizes librsync. To begin with I had `AC_CHECK_HEADERS([librsync_export.h], [], AC_MSG_ERROR(Cannot find librsync))` in configure.ac. This file did not exist in earlier versions of librync. Hence, I upgraded the Ubuntu platforms in GitHub CI to make configure work. Later a realized that I should remove that line enirely to make CFEngine compatible with earlier versions. However, it does not hurt to upgrade the Ubuntu platforms, so decided on keeping this commit. Ticket: None Changelog: None Signed-off-by: Lars Erik Wik <[email protected]>
After upgrading GitHub CI platorms to Ubuntu 24, the ASAN Unit Tests started complaining about One Definition Rule (ODR) violations. Hence, I disabled ODR and created a follow-up ticket (see CFE-4454) to fix the violations and enable ODR again. Ticket: None Changelog: None Signed-off-by: Lars Erik Wik <[email protected]>
Ticket: ENT-12414 Changelog: None Signed-off-by: Lars Erik Wik <[email protected]>
Added API for streaming file contents over the network using the RSYNC algorithm from librsync. Ticket: ENT-12414 Changelog: None Signed-off-by: Lars Erik Wik <[email protected]> Co-authored-by: Craig Comstock <[email protected]>
Ticket: ENT-12414 Changelog: Title Signed-off-by: Lars Erik Wik <[email protected]> Co-authored-by: Craig Comstock <[email protected]>
The implementation is based on the previous protocol for `GET <FILENAME>` requests that used sparse files. Ticket: ENT-12414 Changelog: Title Signed-off-by: Lars Erik Wik <[email protected]>
Apparently there are functions called `SendMessage()` and `GetFileSize()` in the Win32 API. I fixed these name conflicts by adding a `Protocol` prefix to all functions related to the simple network protocol for file stream communication. Furthermore, I changed the name of `GetFileSize()` to `GetSizeOfFile()`. Ticket: ENT-12414 Changelog: None Signed-off-by: Lars Erik Wik <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
TODO:
librsyncas dependency in CI