Skip to content

systemtests: make them usably with Windows binaries on Linux using Wine#1294

Closed
joergsteffens wants to merge 34 commits intobareos:masterfrom
joergsteffens:dev/joergs/master/windows
Closed

systemtests: make them usably with Windows binaries on Linux using Wine#1294
joergsteffens wants to merge 34 commits intobareos:masterfrom
joergsteffens:dev/joergs/master/windows

Conversation

@joergsteffens
Copy link
Member

@joergsteffens joergsteffens commented Oct 25, 2022 •
edited
Loading

Thank you for contributing to the Bareos Project!

Make the Windows binaries testable via systemtests on Linux using Wine.

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR
  • bareos-check-sources --since-merge does not report any problems
Tests
  • Decision taken that a test is required (if not, then remove this paragraph)
  • The choice of the type of test (unit test or systemtest) is reasonable
  • Testname matches exactly what is being tested
  • On a fail, output of the test leads quickly to the origin of the fault

arogge
arogge reviewed Oct 25, 2022
View reviewed changes
@joergsteffens joergsteffens force-pushed the dev/joergs/master/windows branch from fd6c0e7 to 40a55b8 Compare November 2, 2022 08:04
@joergsteffens joergsteffens force-pushed the dev/joergs/master/windows branch 9 times, most recently from c541a73 to 79fe7ae Compare November 14, 2022 08:20
@joergsteffens joergsteffens force-pushed the dev/joergs/master/windows branch 3 times, most recently from c805141 to 08fa1f1 Compare November 21, 2022 10:07
@joergsteffens joergsteffens force-pushed the dev/joergs/master/windows branch from bb27525 to 70857ee Compare November 22, 2022 19:24
@joergsteffens joergsteffens marked this pull request as ready for review November 22, 2022 19:25
@joergsteffens joergsteffens force-pushed the dev/joergs/master/windows branch from ebca217 to 26a8fe7 Compare November 23, 2022 19:03
@joergsteffens joergsteffens requested a review from pstorz November 25, 2022 10:07
@pstorz pstorz added the nobuild label Dec 1, 2022
@joergsteffens joergsteffens mentioned this pull request Dec 19, 2022
Merged
13 tasks
@pstorz pstorz removed the nobuild label Dec 22, 2022
This was referenced Feb 15, 2023
Merged
Merged
@joergsteffens joergsteffens force-pushed the dev/joergs/master/windows branch from b4f4272 to 8f330f5 Compare March 9, 2023 17:41
@joergsteffens joergsteffens force-pushed the dev/joergs/master/windows branch from 6157f65 to b4383b9 Compare March 26, 2023 11:56
@joergsteffens joergsteffens force-pushed the dev/joergs/master/windows branch from b4383b9 to 1971e3f Compare March 26, 2023 12:03
@joergsteffens joergsteffens marked this pull request as ready for review March 26, 2023 12:04
pstorz
pstorz requested changes Mar 27, 2023
View reviewed changes
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

That this PR alters more thant 600 .in files just to add a prefix to an exisiting variable is not a good solution. Probably the variables need to be prefixed themselves by cmake instead.

Other than that please see review comments.

systemtests/cmake/BareosSystemtestFunctions.cmake
get_target_property("binary_dir" "${binary_name}" BINARY_DIR)
# FIXME: is there a better way to get the resulting binary name/path?
if(${CMAKE_SYSTEM_NAME} MATCHES "Windows")
set("${binary_name_to_test_upcase}" "${binary_dir}/${binary_name}.exe")
Copy link
Member

@pstorz pstorz Mar 27, 2023

Choose a reason for hiding this comment

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

Is this required? I would expect that the .exe can be left away and it still starts the executable.

systemtests/scripts/bareos-wine-wrapper.sh
# Tested on Ubuntu 20.04 (wine-7.19), Fedora 35 (wine-6.18).
# While it seams to connect, no input prompt appears.
# Therefore we use the bconsole command (hopefully) installed on the system.
# However, there are reports it works with Fedora 36 (wine-7.18, wine-7.18-2.fc36.x86_64).
Copy link
Member

@pstorz pstorz Mar 27, 2023

Choose a reason for hiding this comment

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

If it does work with Fedora 36 (which we have now), we could avoid all the extra tricks here?

@joergsteffens
Copy link
Member Author

joergsteffens commented Mar 27, 2023

That this PR alters more thant 600 .in files just to add a prefix to an exisiting variable is not a good solution. Probably the variables need to be prefixed themselves by cmake instead.

The problem is, that we need this variable in 2 variants:

  • Unix path format (for inside cmake and shell scripts)
  • Windows path format. When used as Bareos configuration, as the Daemon do run in Windows/Wine. In Bareos we can use the same path, but must prefix it with @wine_drive_prefix@.

If using @wine_drive_prefix@ in the Bareos configuration files is not an option, I still have to create a second variable for every path variable used in the Bareos configuration. Example:

Instead of using

  • @archivedir@ and
  • @wine_drive_prefix@@archivedir@
    I could create a second variable to be used in the Bareos configuration:
  • @archivedir@ and
  • @bcfg_archivedir@ (= @wine_drive_prefix@@archivedir@)

Still, the same amount if *.in files must be modified. Do you think, the second variant is better? Would you agree to it?

1 similar comment
@joergsteffens
Copy link
Member Author

joergsteffens commented Mar 27, 2023

That this PR alters more thant 600 .in files just to add a prefix to an exisiting variable is not a good solution. Probably the variables need to be prefixed themselves by cmake instead.

The problem is, that we need this variable in 2 variants:

  • Unix path format (for inside cmake and shell scripts)
  • Windows path format. When used as Bareos configuration, as the Daemon do run in Windows/Wine. In Bareos we can use the same path, but must prefix it with @wine_drive_prefix@.

If using @wine_drive_prefix@ in the Bareos configuration files is not an option, I still have to create a second variable for every path variable used in the Bareos configuration. Example:

Instead of using

  • @archivedir@ and
  • @wine_drive_prefix@@archivedir@
    I could create a second variable to be used in the Bareos configuration:
  • @archivedir@ and
  • @bcfg_archivedir@ (= @wine_drive_prefix@@archivedir@)

Still, the same amount if *.in files must be modified. Do you think, the second variant is better? Would you agree to it?

joergsteffens and others added 2 commits March 27, 2023 16:02
@joergsteffens @pstorz
Update systemtests/scripts/bareos-wine-wrapper.sh
1123fbf 
Co-authored-by: Philipp Storz <philipp.storz@bareos.com>
@joergsteffens @pstorz
Update systemtests/cmake/BareosSystemtestFunctions.cmake
e52516f 
Co-authored-by: Philipp Storz <philipp.storz@bareos.com>
@bruno-at-bareos
Copy link
Contributor

bruno-at-bareos commented Mar 19, 2024

Closing, a try will be make to run test in native builds.

@joergsteffens joergsteffens deleted the dev/joergs/master/windows branch September 25, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arogge arogge arogge left review comments

@pstorz pstorz pstorz requested changes

Assignees

@pstorz pstorz

Labels

jenkins_repotest_nodebug nobuild onhold

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@joergsteffens @bruno-at-bareos @pstorz @arogge