Skip to content

[roottest] enable several root/aclic tests#19569

Closed
linev wants to merge 6 commits intoroot-project:masterfrom
linev:roottest_aclic
Closed

[roottest] enable several root/aclic tests#19569
linev wants to merge 6 commits intoroot-project:masterfrom
linev:roottest_aclic

Conversation

@linev
Copy link
Member

@linev linev commented Aug 7, 2025

  1. Implement new version of aclic/nolinkdep tests. Seems to be they were testsed before with CINT and were not working with cling. Now at least basic tests are works. Plus linking between script2.C and single.C. Also renaming of script2.C into script.C is testing
  2. Enable again aclic/offset test, was completely skipped
  3. Remove many .rootrc file which are now not necessary

Copy link
Member

@bellenot bellenot left a comment

Choose a reason for hiding this comment

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

Again, I'm not familiar enough with these tests. Let's ask @pcanal what he thinks about this PR

@linev linev requested review from bellenot and removed request for dpiparo August 7, 2025 14:27
@github-actions
Copy link

github-actions bot commented Aug 7, 2025

Test Results

    18 files      18 suites   3d 3h 18m 49s ⏱️
 3 668 tests  3 624 ✅   0 💤 44 ❌
64 436 runs  64 146 ✅ 246 💤 44 ❌

For more details on these failures, see this check.

Results for commit 68da512.

♻️ This comment has been updated with latest results.

@linev linev closed this Aug 22, 2025
@linev linev reopened this Aug 22, 2025
FIXTURES_SETUP root-aclic-nolinkdep-load1-fixture)

ROOTTEST_ADD_TEST(load2
MACRO script2.C+
Copy link
Member

@pcanal pcanal Aug 28, 2025

Choose a reason for hiding this comment

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

I am unclear whether the intent was to run this script with or without script1_C being there ... but did need to have single_C present (as opposed to the running of script1).

FIXTURES_SETUP root-aclic-nolinkdep-single-fixture)

ROOTTEST_ADD_TEST(load1
MACRO script1.C+
Copy link
Member

Choose a reason for hiding this comment

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

The corresponding makefile like was:

load1b.log: load1.log load2b.log
	$(CMDECHO) rm -f single_C.$(DllSuf); root.exe -b -l -q script1.C+ > load1b.log 2>&1

i.e. this step was meant to be run without single_C being present.

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems to be logic in "nolingdep" more complicated.
There is rootlogon.C script which changes linker flags.
So one should not try to link libraries with each other but check if changed linker flags allow to run such compiled macros without having some symbols defined.
I will adjust cmake once again

Copy link
Member

Choose a reason for hiding this comment

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

There is rootlogon.C script which changes linker flags.

I missed/forgot about that. So actually the whole point of the test is to make sure that ACLiC is following the request of those flags (they are all meant to force ACliC to only keep the library actually used independently of how ROOT was compiled.

So one should not try to link libraries with each other but

In the contrary it is the whole point, i.e. to link the libraries with each other and check/make-sure that the 'useless' ones have been dropped.

without having some symbols defined.

This is not the goal. Rather testing 'allow missing symbols", this test is checking dead_strip_dylibs and -Wl,--as-needed which is about ignoring libraries that do not actually provide symbols.

Copy link
Member Author

Choose a reason for hiding this comment

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

In current version I fully replicate logic in GNUMakefile.
So I did not try extra compile macro before calling link.C script.
And then remove in-between single_C.so - as it was done in GNUMakefile.

Question - should we add ROOTTEST_COMPILE_MACRO before?

Copy link
Member

Choose a reason for hiding this comment

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

Question - should we add ROOTTEST_COMPILE_MACRO before?

We have to and/or re-introduce the link(1) part.

Copy link
Member Author

Choose a reason for hiding this comment

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

Trying to run original GNUMakefile failing.
It fails with load2.log with following statement:

Processing script2.C+...
In file included from script2_C_ACLiC_dict dictionary payload:8:
././script2.C:9:6: error: redefinition of 'script'
void script() {
     ^
././script1.C:7:6: note: previous definition is here
void script() {
     ^
Warning in <TInterpreter::TCling::RegisterModule>: Problems declaring payload for module script2_C_ACLiC_dict.
input_line_16:2:3: error: use of undeclared identifier 'script2'
 (script2())
  ^
Error in <HandleInterpreterException>: Error evaluating expression (script2())
Execution of your code was aborted.

That's why I change original link.C script - it was not working.

So I propose to remove aclic/nolinkdep test from the PR and one can try to activate it again separately.

linev added 5 commits August 29, 2025 08:28
.rootrc generated automatically if missing
With CLING some old features no longer working,
but at least one can compile and run existing scripts
And also renaming of scripts works as before
Here macro with incomplete symbols testes, which requires changes in linker flags
Try fully reproduce dependency from makefiles - which now can be deleted
Building shared library with missing symbols does not work on windows
Probably one can manipulate linker flags via rootlogon.C script -
as it done for Linux and Mac
@linev linev closed this Sep 5, 2025
@linev linev reopened this Sep 5, 2025
Comment on lines +1 to +29
# building shared library with missing symbols does not work on windows
# probably one can manipulate linker flags via rootlogon.C script - as it done for Linux and Mac
if(MSVC AND NOT win_broken_tests)
return()
endif()

ROOTTEST_ADD_TEST(build1
MACRO link.C
PRECMD ${CMAKE_COMMAND} -E remove "script1_C.* script2_C.*"
COPY_TO_BUILDDIR single.C script.C script1.C script2.C rootlogon.C
FIXTURES_SETUP root-aclic-nolinkdep-build1-fixture)

ROOTTEST_ADD_TEST(single
MACRO ${CMAKE_CURRENT_BINARY_DIR}/single.C+
FIXTURES_REQUIRED root-aclic-nolinkdep-build1-fixture
FIXTURES_SETUP root-aclic-nolinkdep-single-fixture)


ROOTTEST_ADD_TEST(load1
MACRO ${CMAKE_CURRENT_BINARY_DIR}/script1.C+
FIXTURES_REQUIRED root-aclic-nolinkdep-build1-fixture
root-aclic-nolinkdep-single-fixture
FIXTURES_SETUP root-aclic-nolinkdep-load1-fixture)

ROOTTEST_ADD_TEST(load2
MACRO ${CMAKE_CURRENT_BINARY_DIR}/script2.C+
FIXTURES_REQUIRED root-aclic-nolinkdep-build1-fixture
root-aclic-nolinkdep-single-fixture
FIXTURES_SETUP root-aclic-nolinkdep-load2-fixture)
Copy link
Member

Choose a reason for hiding this comment

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

ROOTTEST_COMPILE_MACRO(single_C
# except that this is not yet support by COMPILE_MACRO
PRECMD ${CMAKE_COMMAND} -E remove "script1_C.* script2_C.*"
FIXTURES_SETUP root-aclic-nolinkdep-build1-single-fixture)

ROOTTEST_COMPILE_MACRO(script1_C
BUILDLIB single_C
FIXTURES_REQUIRED root-aclic-nolinkdep-build1-single-fixture
FIXTURES_SETUP root-aclic-nolinkdep-build1-script1-fixture)

ROOTTEST_COMPILE_MACRO(script2_C
BUILDLIB single_C script1_C
FIXTURES_REQUIRED root-aclic-nolinkdep-build1-single-fixture
FIXTURES_REQUIRED root-aclic-nolinkdep-build1-script1-fixture
FIXTURES_SETUP root-aclic-nolinkdep-build1-script2-fixture)

ROOTTEST_ADD_TEST(single
MACRO ${CMAKE_CURRENT_BINARY_DIR}/single.C+
FIXTURES_REQUIRED root-aclic-nolinkdep-build1-script2-fixture
FIXTURES_SETUP root-aclic-nolinkdep-single-fixture)

@linev
Copy link
Member Author

linev commented Sep 10, 2025

Close in favor of #19857 and #19856

@linev linev closed this Sep 10, 2025
@linev linev deleted the roottest_aclic branch September 11, 2025 05:58
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.

3 participants