Skip to content

GH-48025: [C++][GLib] Replace instances where build path is being added to built artifacts#48026

Merged
raulcd merged 3 commits into
apache:mainfrom
raulcd:GH-48025
Nov 3, 2025
Merged

GH-48025: [C++][GLib] Replace instances where build path is being added to built artifacts#48026
raulcd merged 3 commits into
apache:mainfrom
raulcd:GH-48025

Conversation

@raulcd

@raulcd raulcd commented Nov 3, 2025

Copy link
Copy Markdown
Member

Rationale for this change

There are several places where we are adding the build path on our built artifacts. We should avoid that in order to improve reproducibility of our builds.

What changes are included in this PR?

General build system hygiene improvement redacting absolute paths from the compiler flags to improve build reproducibility and caching efficiency.

Update for mkenums from full @filename@ to @basename@ as seen on their official documentation:

The base name of the input file currently being processed (e.g. foo.h). Typically you want to use @basename@ in place of @filename@ in your templates, to improve the reproducibility of the build. (Since: 2.22)

Are these changes tested?

Validated locally that reprotest doesn't fail on enums anymore.

CI and tests have run.

Are there any user-facing changes?

No

@github-actions

github-actions Bot commented Nov 3, 2025

Copy link
Copy Markdown

⚠️ GitHub issue #48025 has been automatically assigned in GitHub to PR creator.

@raulcd

raulcd commented Nov 3, 2025

Copy link
Copy Markdown
Member Author

As far as I understand failures are unrelated. Examples:

I am unsure on what is the problem on C GLib & Ruby / AMD64 Ubuntu 22.04 GLib & Ruby but does not seem related:

 + ruby test/run-test.rb
/arrow/c_glib /
/var/lib/gems/3.0.0/gems/test-unit-3.7.0/lib/test/unit/util/backtracefilter.rb:16:in `method': undefined method `start' for class `#<Class:PowerAssert>' (NameError)
	from /var/lib/gems/3.0.0/gems/test-unit-3.7.0/lib/test/unit/util/backtracefilter.rb:16:in `<module:BacktraceFilter>'
	from /var/lib/gems/3.0.0/gems/test-unit-3.7.0/lib/test/unit/util/backtracefilter.rb:9:in `<module:Util>'
	from /var/lib/gems/3.0.0/gems/test-unit-3.7.0/lib/test/unit/util/backtracefilter.rb:8:in `<module:Unit>'
	from /var/lib/gems/3.0.0/gems/test-unit-3.7.0/lib/test/unit/util/backtracefilter.rb:7:in `<module:Test>'
	from /var/lib/gems/3.0.0/gems/test-unit-3.7.0/lib/test/unit/util/backtracefilter.rb:6:in `<top (required)>'
	from /var/lib/gems/3.0.0/gems/test-unit-3.7.0/lib/test/unit/assertions.rb:7:in `require_relative'
	from /var/lib/gems/3.0.0/gems/test-unit-3.7.0/lib/test/unit/assertions.rb:7:in `<top (required)>'
	from /var/lib/gems/3.0.0/gems/test-unit-3.7.0/lib/test/unit/testcase.rb:12:in `require_relative'
	from /var/lib/gems/3.0.0/gems/test-unit-3.7.0/lib/test/unit/testcase.rb:12:in `<top (required)>'
	from /var/lib/gems/3.0.0/gems/test-unit-3.7.0/lib/test/unit/autorunner.rb:7:in `require_relative'
	from /var/lib/gems/3.0.0/gems/test-unit-3.7.0/lib/test/unit/autorunner.rb:7:in `<top (required)>'
	from <internal:/usr/lib/ruby/vendor_ruby/rubygems/core_ext/kernel_require.rb>:96:in `require'
	from <internal:/usr/lib/ruby/vendor_ruby/rubygems/core_ext/kernel_require.rb>:96:in `require'
	from test/run-test.rb:113:in `<main>'

@raulcd raulcd marked this pull request as ready for review November 3, 2025 09:51
@raulcd raulcd requested a review from kou as a code owner November 3, 2025 09:51
@kou

kou commented Nov 3, 2025

Copy link
Copy Markdown
Member

The "undefined method" error will be fixed soon in upstream: test-unit/test-unit#329

@kou kou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Comment thread cpp/src/arrow/util/config_internal.h.cmake
@kou kou changed the title GH-48025: [C++][CGLib] Replace instances where build path is being added to built artifacts GH-48025: [C++][GLib] Replace instances where build path is being added to built artifacts Nov 3, 2025
@github-actions github-actions Bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels Nov 3, 2025
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
@raulcd raulcd merged commit 534ef71 into apache:main Nov 3, 2025
41 of 44 checks passed
@raulcd raulcd removed the awaiting merge Awaiting merge label Nov 3, 2025
@raulcd raulcd deleted the GH-48025 branch November 3, 2025 11:23
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 0 benchmarking runs that have been run so far on merge-commit 534ef71.

None of the specified runs were found on the Conbench server.

The full Conbench report has more details.

Mottl pushed a commit to Mottl/arrow that referenced this pull request May 26, 2026
…ng added to built artifacts (apache#48026)

### Rationale for this change

There are several places where we are adding the build path on our built artifacts. We should avoid that in order to improve reproducibility of our builds.

### What changes are included in this PR?

General build system hygiene improvement redacting absolute paths from the compiler flags to improve build reproducibility and caching efficiency.

Update for mkenums from full `@ filename@` to `@ basename@` as seen on their official documentation:

> The base name of the input file currently being processed (e.g. foo.h). Typically you want to use `@ basename@` in place of `@ filename@` in your templates, to improve the reproducibility of the build. (Since: 2.22) 

### Are these changes tested?

Validated locally that reprotest doesn't fail on `enums` anymore.

CI and tests have run.

### Are there any user-facing changes?

No

* GitHub Issue: apache#48025

Lead-authored-by: Raúl Cumplido <raulcumplido@gmail.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants