Skip to content

Add "touch" operation#36

Open
cottsay wants to merge 2 commits intomainfrom
cottsay/repo-touch
Open

Add "touch" operation#36
cottsay wants to merge 2 commits intomainfrom
cottsay/repo-touch

Conversation

@cottsay
Copy link
Copy Markdown
Member

@cottsay cottsay commented Mar 27, 2026

This operation is intended to function similar to the "touch" command and will essentially bypass the optimizations which avoid modifying repository metadata when there are no effective changes to it.

It is particularly useful for using createrepo-agent to generate initial metadata for an empty repository without seeding it with any packages.

The actual effect of this change happens in repo_cache.c. All the rest of the changes are plumbing and tests.

This operation is intended to function similar to the "touch" command
and will essentially bypass the optimizations which avoid modifying
repository metadata when there are no effective changes to it.

It is particularly useful for using createrepo-agent to generate initial
metadata for an empty repository without seeding it with any packages.
@cottsay cottsay self-assigned this Mar 27, 2026
@cottsay cottsay added the enhancement New feature or request label Mar 27, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 27, 2026

Codecov Report

❌ Patch coverage is 79.41176% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.75%. Comparing base (955efe9) to head (e47801a).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/createrepo-agent/command.c 68.18% 3 Missing and 4 partials ⚠️
src/createrepo-cache/coordinator.c 66.66% 1 Missing and 2 partials ⚠️
src/createrepo-cache/repo_cache.c 81.81% 1 Missing and 1 partial ⚠️
src/python/client.c 87.50% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #36      +/-   ##
==========================================
+ Coverage   48.97%   49.75%   +0.78%     
==========================================
  Files          20       20              
  Lines        2638     2705      +67     
  Branches      538      555      +17     
==========================================
+ Hits         1292     1346      +54     
- Misses       1039     1044       +5     
- Partials      307      315       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@j-rivero
Copy link
Copy Markdown

j-rivero commented Apr 7, 2026

Generally looks fine to my eyes. Some questions and probably considering add more tests for corner cases: no-valid-architecture argument, no-argument, single architecture passed.

It is particularly useful for using createrepo-agent to generate initial metadata for an empty repository without seeding it with any packages.

Is this something you plan to add to the code automatically when creating the initial metadata or must be called by the user in a different command?

@j-rivero
Copy link
Copy Markdown

j-rivero commented Apr 7, 2026

Other minor problem. If I run mkdir build, cd build, cmake .., make tests there is a problem on smake.touch/bar:

  ⎿  Error: Exit code 1                                                                                                                                                                       
     Note: Google Test filter = smoke/smoke.touch/bare                                                                                                                                        
     [==========] Running 1 test from 1 test suite.                                                                                                                                           
     [----------] Global test environment set-up.                                                                                                                                             
     [----------] 1 test from smoke/smoke                                                                                                                                                     
     [ RUN      ] smoke/smoke.touch/bare                                                                                                                                                      
     /home/jrivero/code/infra/createrepo-agent/test/integration_utils.hpp:46: Failure                                                                                                         
     Value of: fs::is_directory(fixture)                                                                                                                                                      
       Actual: false                                                                                                                                                                          
     Expected: true 

Seems like tests must be called from build/tests or running ctest directly?

@cottsay
Copy link
Copy Markdown
Member Author

cottsay commented Apr 7, 2026

Thanks for taking a look.

considering add more tests for corner cases: no-valid-architecture argument, no-argument, single architecture passed.

Good call out, I'll add them.

Seems like tests must be called from build/tests or running ctest directly?

I can't reproduce this. It seems to pass for me when I run mkdir build && cd build && cmake .. && make -j5 && make test.

The fixture directory should be populated as part of the CMake configure step.

@cottsay
Copy link
Copy Markdown
Member Author

cottsay commented Apr 7, 2026

Seems like tests must be called from build/tests or running ctest directly?

I can't reproduce this. It seems to pass for me when I run mkdir build && cd build && cmake .. && make -j5 && make test.

The fixture directory should be populated as part of the CMake configure step.

I managed to hit this! It's only the bare test that's failing. For some reason, the empty(-ish) bare fixture directory doesn't get copied into the staging directory during the configure phase.

Maybe I'll add an explicit CTest fixture that copies the fixtures into the testing directory fresh as part of each invocation.

@cottsay cottsay force-pushed the cottsay/repo-touch branch 2 times, most recently from 9c8a4a0 to 5a53020 Compare April 7, 2026 20:46
@cottsay cottsay force-pushed the cottsay/repo-touch branch from 5a53020 to e47801a Compare April 7, 2026 20:49
@j-rivero
Copy link
Copy Markdown

j-rivero commented Apr 8, 2026

I managed to hit this! It's only the bare test that's failing. For some reason, the empty(-ish) bare fixture directory doesn't get copied into the staging directory during the configure phase.

great, thanks !

I think that I found a segfault happening in the tests. I've asked my debugging agent to investigate the gdb of it and the explanation that it produced sounds reasonable to me although I'm not familiar with the code base to fully verified the claims. Black-box testing the fix worked.

for i in $(seq 1 50); do ./test_smoke --gtest_filter=smoke/smoke.touch/bare ; done
...
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from smoke/smoke
[ RUN      ] smoke/smoke.touch/bare
CSegmentation fault (core dumped)

The problem

smoke/smoke.touch/bare crashes non-deterministically inside libxml2 (xmlDocDumpFormatMemoryEnc) when called from worker threads in cra_repomd_flush_worker at src/createrepo-cache/repo_cache.c:1559.

Stack trace from the crash:

Thread 23 "pool" received signal SIGSEGV:
#0  libxml2.so.2 (inside xmlDocDumpFormatMemoryEnc)
#1  xmlDocDumpFormatMemoryEnc
#2  cr_xml_dump_repomd  (libcreaterepo_c)
#3  cra_xml_write_repomd
#4  cra_repomd_flush_worker

libxml2's documentation xmlInitParser() must be called from the main thread before any other threads are created.

Why only bare reproduces it:

  • empty / populated fixtures already have a repomd.xml on disk, so cra_repo_cache_load()cr_xml_parse_repomd() runs on the main thread during realize and indirectly initializes libxml2 globals before any workers spawn.
  • bare has no repodata, so cra_repo_cache_realize (repo_cache.c:909) short-circuits and never touches libxml2 on the main thread.
  • touch aarch64 x86_64 then dirties 5 repos (SRPMS + arch + debug × 2). cra_cache_flush spawns g_get_num_processors() workers that all race into libxml2's first-time init simultaneously → SIGSEGV.

Why it's non-deterministic: the race only fires when two workers hit the lazy init window at the same time. Single-core scheduling, CPU caches, or cold cache for libxml2's encoding tables all affect the outcome.

Why the Python test test_touch didn't catch it: the Python extension is loaded into an interpreter that has already pulled libxml2 state through other startup paths, papering over the race.

A possible fix

Added a call to cr_xml_dump_init() (createrepo_c's thin wrapper around xmlInitParser(), specifically provided for this purpose) at the top of cra_cache_new() in src/createrepo-cache/repo_cache.c:422. It runs on the main thread before any flush worker pool is created, is idempotent, and requires no new dependency since createrepo_c is already linked.

diff --git a/src/createrepo-cache/repo_cache.c b/src/createrepo-cache/repo_cache.c
index 8c279ba..5d2bacb 100644
--- a/src/createrepo-cache/repo_cache.c
+++ b/src/createrepo-cache/repo_cache.c
@@ -424,6 +424,11 @@ cra_cache_new(const char * path)
   cra_Cache * cache;
   gpgme_error_t rc;
 
+  // Initialize libxml2 on the main thread. Without this, concurrent first-use
+  // from flush worker threads races on libxml2's lazy global init and crashes
+  // inside xmlDocDumpFormatMemoryEnc. The initialization is idempotent.
+  cr_xml_dump_init();
+
   cache = g_new0(cra_Cache, 1);
   if (!cache) {
     return NULL;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants