Minimal backport to support libnfs 6 on v0.23 #2198
Conversation
This is based on the following commits in master: - MusicPlayerDaemon@31e583e - MusicPlayerDaemon@58e3b83 also changed cpp_std=c++20 to facilitate the use of span
BlackIkeEagle
force-pushed
the
v0.23-libnfs6-backport
branch
from
c54ecfc to
913e1ac
Compare
|
libnfs 6 is still blocked by the upper limit introdcued in c48dbd5 resulting in: |
This reverts commit c48dbd5.
|
Raising the C++ version requires also raising the minimum compiler version and Meson version. We should not do that for stable versions, because it means some users cannot get bug fixes anymore. |
|
This PR is mainly there to see if the backport looks fine to apply for use in Arch Linux for example. Usually "stable" distro's will also not yet have the libnfs v6 package in their repo's. |
|
Initially I did not want to even bother you with this because of #2165 (comment). But the initial backport I took from homebrew failed hard, so thats where this PR comes from |
Was the intent to only raise the C++ standard if libnfs v6 is being used? As that currently is not the case. @MaxKellermann if the use of std::span was removed so the C++ standard does not need to be increased you would then not be against this pull request on principal? |
|
I fear making C++20 conditional based on a library version check will be complicated. Does GCC/libstdc++ allow using std::span in C++17 mode? If yes, then nobody's MPD would break: people with libnfs<6 (with old GCC) see no change, and people who use libnfs6 need a GCC version that is recent enough. If the code was changed to not use C++20 features, it could be merged. In MPD 0.23, we have the |
Sadly no it produces undeclared errors as expected:
This is probably very wrong as it is only compile tested after converting to diff --git a/src/lib/nfs/Connection.cxx b/src/lib/nfs/Connection.cxx
index c9d7985f7..71612500b 100644
--- a/src/lib/nfs/Connection.cxx
+++ b/src/lib/nfs/Connection.cxx
@@ -103,11 +103,25 @@ NfsConnection::CancellableCallback::Stat(nfs_context *ctx,
inline void
NfsConnection::CancellableCallback::Read(nfs_context *ctx, struct nfsfh *fh,
- uint64_t offset, size_t size)
+ uint64_t offset,
+#ifdef LIBNFS_API_2
+ WritableBuffer<std::byte> dest
+#else
+ std::size_t size
+#endif
+ )
{
assert(connection.GetEventLoop().IsInside());
- int result = nfs_pread_async(ctx, fh, offset, size, Callback, this);
+ int result = nfs_pread_async(ctx, fh,
+#ifdef LIBNFS_API_2
+ dest.data, dest.size,
+#endif
+ offset,
+#ifndef LIBNFS_API_2
+ size,
+#endif
+ Callback, this);
if (result < 0)
throw FormatRuntimeError("nfs_pread_async() failed: %s",
nfs_get_error(ctx));
@@ -329,7 +343,12 @@ NfsConnection::Stat(struct nfsfh *fh, NfsCallback &callback)
}
void
-NfsConnection::Read(struct nfsfh *fh, uint64_t offset, size_t size,
+NfsConnection::Read(struct nfsfh *fh, uint64_t offset,
+#ifdef LIBNFS_API_2
+ WritableBuffer<std::byte> dest,
+#else
+ std::size_t size,
+#endif
NfsCallback &callback)
{
assert(GetEventLoop().IsInside());
@@ -337,7 +356,13 @@ NfsConnection::Read(struct nfsfh *fh, uint64_t offset, size_t size,
auto &c = callbacks.Add(callback, *this, false);
try {
- c.Read(context, fh, offset, size);
+ c.Read(context, fh, offset,
+#ifdef LIBNFS_API_2
+ dest
+#else
+ size
+#endif
+ );
} catch (...) {
callbacks.Remove(c);
throw;
diff --git a/src/lib/nfs/Connection.hxx b/src/lib/nfs/Connection.hxx
index 49987e2c3..4c5cb9c05 100644
--- a/src/lib/nfs/Connection.hxx
+++ b/src/lib/nfs/Connection.hxx
@@ -24,6 +24,7 @@
#include "event/SocketEvent.hxx"
#include "event/CoarseTimerEvent.hxx"
#include "event/DeferEvent.hxx"
+#include "util/WritableBuffer.hxx"
#include <string>
#include <list>
@@ -71,7 +72,13 @@ class NfsConnection {
void Open(nfs_context *context, const char *path, int flags);
void Stat(nfs_context *context, struct nfsfh *fh);
void Read(nfs_context *context, struct nfsfh *fh,
- uint64_t offset, size_t size);
+ uint64_t offset,
+#ifdef LIBNFS_API_2
+ WritableBuffer<std::byte> dest
+#else
+ std::size_t size
+#endif
+ );
/**
* Cancel the operation and schedule a call to
@@ -193,7 +200,12 @@ public:
/**
* Throws std::runtime_error on error.
*/
- void Read(struct nfsfh *fh, uint64_t offset, size_t size,
+ void Read(struct nfsfh *fh, uint64_t offset,
+#ifdef LIBNFS_API_2
+ WritableBuffer<std::byte> dest,
+#else
+ std::size_t size,
+#endif
NfsCallback &callback);
void Cancel(NfsCallback &callback) noexcept;
diff --git a/src/lib/nfs/FileReader.cxx b/src/lib/nfs/FileReader.cxx
index 6e9457d79..6bd75e9ea 100644
--- a/src/lib/nfs/FileReader.cxx
+++ b/src/lib/nfs/FileReader.cxx
@@ -129,7 +129,15 @@ NfsFileReader::Read(uint64_t offset, size_t size)
{
assert(state == State::IDLE);
+#ifdef LIBNFS_API_2
+ assert(!read_buffer);
+ // TOOD read into caller-provided buffer
+ read_buffer = std::make_unique<std::byte[]>(size);
+ connection->Read(fh, offset, {read_buffer.get(), size}, *this);
+#else
connection->Read(fh, offset, size, *this);
+#endif
+
state = State::READ;
}
@@ -137,7 +145,12 @@ void
NfsFileReader::CancelRead() noexcept
{
if (state == State::READ) {
+#ifdef LIBNFS_API_2
+ assert(read_buffer);
+ read_buffer.release();
+#endif
connection->Cancel(*this);
+
state = State::IDLE;
}
}
diff --git a/src/lib/nfs/FileReader.hxx b/src/lib/nfs/FileReader.hxx
index 8d257efdd..a2b9e9c60 100644
--- a/src/lib/nfs/FileReader.hxx
+++ b/src/lib/nfs/FileReader.hxx
@@ -32,6 +32,10 @@
#include <sys/stat.h>
+#ifdef LIBNFS_API_2
+#include <memory>
+#endif
+
struct nfsfh;
class NfsConnection;
@@ -68,6 +72,10 @@ class NfsFileReader : NfsLease, NfsCallback {
*/
InjectEvent defer_open;
+#ifdef LIBNFS_API_2
+ std::unique_ptr<std::byte[]> read_buffer;
+#endif
+
public:
NfsFileReader() noexcept;
~NfsFileReader() noexcept;
diff --git a/src/lib/nfs/meson.build b/src/lib/nfs/meson.build
index 274ee7b47..586a71391 100644
--- a/src/lib/nfs/meson.build
+++ b/src/lib/nfs/meson.build
@@ -1,9 +1,16 @@
-nfs_dep = dependency('libnfs', version: ['>= 4', '< 6'], required: get_option('nfs'))
+nfs_dep = dependency('libnfs', version: '>= 4', required: get_option('nfs'))
conf.set('ENABLE_NFS', nfs_dep.found())
if not nfs_dep.found()
subdir_done()
endif
+if nfs_dep.version().version_compare('>=6')
+ # libnfs has no version macro therefore we must detect the API
+ # version 2 at configure time
+ nfs_dep = declare_dependency(compile_args: '-DLIBNFS_API_2',
+ dependencies: nfs_dep)
+endif
+
nfs = static_library(
'nfs',
'Connection.cxx', |
This is based on the following commits in master: