Hi Jun,
Thanks for testing!
Den tors 26 juni 2025 kl 05:02 skrev Jun Omae <jun6...@gmail.com>:
> Hi,
>
> On 2025/06/21 23:38, Timofei Zhakov wrote:
> >> In my testing, the whole action completed successfully on Mac OS, except
> >> of one test:
> >>
> >> ```
> >> 24: svn_tests: E200006: Expected error SVN_ERR_SQLITE_BUSY but got
> SVN_ERR_SQLITE_ERROR
> >> 24: svn_tests: E200030: sqlite[S10]: disk I/O error
> >> 24: svn_tests: E200042: Additional errors:
> >> 24: svn_tests: E200030: sqlite[S10]: disk I/O error
> >> 24: svn_tests: E200044: SQLite transaction rollback failed
> >> 24: svn_tests: E200030: sqlite[S1]: cannot rollback - no transaction is
> active
> >> 24: svn_tests: E200030: sqlite[S1]: cannot rollback - no transaction is
> active
> >> 24: FAIL: fs-test 65: test commit with locked rep-cache
> >> ```
> >>
> >> It seems like the problem is not in the build process, but sqlite works
> not
> >> as we expect on this platform.
> >
> > Any ideas?
>
> This issue occurs when using the bundled libsqlit3.dylib on macOS. It
> doesn't occur with the same version (3.43.2) of sqlite-amalgamation and
> the latest version (3.50.1) installed from brew.
>
> In the workflow, sqlite is installed from brew but it is not used. We
> should add pkgconfig path of sqlite to PKG_CONFIG_PATH.
>
> [[[
> diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml
> index f63b44a61..9b682b7d5 100644
> --- a/.github/workflows/cmake.yml
> +++ b/.github/workflows/cmake.yml
> @@ -149,11 +149,15 @@ jobs:
> # since there is no separate package.
> run: |
> brew install apr apr-util expat sqlite ninja subversion
> - ls -r /opt/homebrew/opt/subversion/libexec/serf
> - $PKG_CONFIG_PATH = "/opt/homebrew/opt/apr/lib/pkgconfig"
> - $PKG_CONFIG_PATH += ":/opt/homebrew/opt/apr-util/lib/pkgconfig"
> - $PKG_CONFIG_PATH += ":/opt/homebrew/opt/expat/lib/pkgconfig"
> - $PKG_CONFIG_PATH +=
> ":/opt/homebrew/opt/subversion/libexec/serf/lib/pkgconfig"
> + $brew_prefix = "$(& brew --prefix)"
> + ls -r "$brew_prefix/opt/subversion/libexec/serf"
> + $PKG_CONFIG_PATH = @(
> + "$brew_prefix/opt/apr/lib/pkgconfig",
> + "$brew_prefix/opt/apr-util/lib/pkgconfig",
> + "$brew_prefix/opt/expat/lib/pkgconfig",
> + "$brew_prefix/opt/sqlite/lib/pkgconfig",
> + "$brew_prefix/opt/subversion/libexec/serf/lib/pkgconfig") `
> + -Join ":"
> "PKG_CONFIG_PATH=$PKG_CONFIG_PATH" >> $env:GITHUB_ENV
>
> - name: Use LF for Git checkout
> ]]]
>
If this makes the compile succeed, I think you should just commit it! It
only relates to the GHA system anyway, not anything we ship.
>
> Also, the searching libraries using pkg-config doesn't work on my
> environment. I think it is caused by the prefix passed to
> pkg_check_modules is wrong.
>
> [[[
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index e60809b1e..d94c30b14 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -269,8 +269,8 @@ if(SVN_USE_PKG_CONFIG)
> # apr-1
> add_library(external-apr ALIAS PkgConfig::apr1)
>
> - pkg_check_modules(aprutil-1 REQUIRED IMPORTED_TARGET apr-util-1)
> - add_library(external-aprutil ALIAS PkgConfig::aprutil-1)
> + pkg_check_modules(aprutil1 REQUIRED IMPORTED_TARGET apr-util-1)
> + add_library(external-aprutil ALIAS PkgConfig::aprutil1)
> else()
> # apr-2
> pkg_check_modules(apr2 REQUIRED IMPORTED_TARGET apr-2)
> ]]]
>
I can't quite wrap my head around the function of the prefix In my
testings, it didn't really seem to matter what the prefix was. Expert
opinions welcome...
>
> To investigate unexpected behavior of libsqlite3, I apply temporarily
> the following patch in order to enable extended result code from
> libsqlite3.
>
> [[[
> diff --git a/subversion/libsvn_subr/sqlite.c
> b/subversion/libsvn_subr/sqlite.c
> index ee6fa16a8..fe0a58326 100644
> --- a/subversion/libsvn_subr/sqlite.c
> +++ b/subversion/libsvn_subr/sqlite.c
> @@ -169,11 +169,11 @@ struct svn_sqlite__value_t
>
>
> /* Convert SQLite error codes to SVN. Evaluates X multiple times */
> -#define SQLITE_ERROR_CODE(x) ((x) == SQLITE_READONLY \
> +#define SQLITE_ERROR_CODE(x) (((x) & 0xFF) == SQLITE_READONLY \
> ? SVN_ERR_SQLITE_READONLY \
> - : ((x) == SQLITE_BUSY \
> + : (((x) & 0xFF) == SQLITE_BUSY \
> ? SVN_ERR_SQLITE_BUSY \
> - : ((x) == SQLITE_CONSTRAINT \
> + : (((x) & 0xFF) == SQLITE_CONSTRAINT \
> ? SVN_ERR_SQLITE_CONSTRAINT \
> : SVN_ERR_SQLITE_ERROR)))
>
> @@ -848,6 +848,9 @@ internal_open(svn_sqlite__db_t *db, const char *path,
> svn_sqlite__mode_t mode,
> somebody initialized SQLite before us it is needed anyway. */
> flags |= SQLITE_OPEN_NOMUTEX;
>
> + /* Require extended result code */
> + flags |= SQLITE_OPEN_EXRESCODE;
> +
> #if !defined(WIN32) && !defined(SVN_SQLITE_INLINE)
> if (mode == svn_sqlite__mode_rwcreate)
> {
> ]]]
>
> After the patch and retry the unit tests, the bundled libsqlite3.dylib
> returns SQLITE_IOERR_LOCK (3850) instead of expected SQLITE_BUSY_*. I
> guess that Apple apply something to libsqlite3 and bundle it.
>
> [[[
> 24: svn_tests: E200006: Expected error SVN_ERR_SQLITE_BUSY but got
> SVN_ERR_SQLITE_ERROR
> 24: svn_tests: E200030: sqlite[S3850]: disk I/O error
> 24: svn_tests: E200042: Additional errors:
> 24: svn_tests: E200030: sqlite[S3850]: disk I/O error
> 24: svn_tests: E200044: SQLite transaction rollback failed
> 24: svn_tests: E200030: sqlite[S1]: cannot rollback - no transaction is
> active
> 24: svn_tests: E200030: sqlite[S1]: cannot rollback - no transaction is
> active
> ]]]
>
Not sure if you intended this to be included in the patch or if it was just
temporary during debugging.
Cheers,
Daniel
>
> With finality, tests on macOS pass by the above patches.
>
> [[[
> 100% tests passed, 0 tests failed out of 123
>
> Total Test time (real) = 246.48 sec
> ]]]
>
> See also:
> *
> https://github.com/apache/subversion/compare/trunk...jun66j5:subversion:macos-sqlite.patch
> *
> https://github.com/jun66j5/subversion/actions/runs/15891503530/job/44814750780
>
> --
> Jun Omae <jun6...@gmail.com> (大前 潤)
>
