Skip to content

winbareos-native.nsi: do not package python3 plugins#2076

Merged
BareosBot merged 10 commits intobareos:masterfrom
pstorz:dev/pstorz/native-installer-no-python3
Dec 31, 2024
Merged

winbareos-native.nsi: do not package python3 plugins#2076
BareosBot merged 10 commits intobareos:masterfrom
pstorz:dev/pstorz/native-installer-no-python3

Conversation

@pstorz
Copy link
Member

@pstorz pstorz commented Dec 17, 2024 •
edited by bruno-at-bareos
Loading

This PR aims to no install python3-*.dll on windows as it is not 100% sure how we want to handle python support on windows.

This PR also removes support for the cross-compiling of bareos for windows.

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)

Make sure you check/merge the PR using devtools/pr-tool to have some simple automated checks run and a proper changelog record added.

General
  • Is the PR title usable as CHANGELOG entry?
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
  • Required backport PRs have been created
  • Correct milestone is set
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

@pstorz pstorz force-pushed the dev/pstorz/native-installer-no-python3 branch from d753e38 to be862dc Compare December 17, 2024 17:37
@bruno-at-bareos bruno-at-bareos self-requested a review December 18, 2024 08:23
@bruno-at-bareos bruno-at-bareos added this to the 25.0.0 milestone Dec 18, 2024
bruno-at-bareos
bruno-at-bareos requested changes Dec 18, 2024
View reviewed changes
Copy link
Contributor

@bruno-at-bareos bruno-at-bareos left a comment

Choose a reason for hiding this comment

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

Thanks for the proposal:

if Director's plugin is fully commented, we still need to fix SD as we want autoxflate but not python3-sd.dll installed.

Thanks to revisit that part.
See also my comment about the two ghost services.

@bruno-at-bareos
Copy link
Contributor

bruno-at-bareos commented Dec 18, 2024

Hi @pstorz while we are fixing this part, in newer (at least native) build we now have those 2 services installed by default (while they are only needed for testing)

Service Name: bareos_sd-accurate-lmdb-stresstest
Path to Executable: C:\ProgramData\chocolatey\lib\NSSM\tools\nssm.exe

Service Name: bareos_sd-bconsole-basic
Path to Executable: C:\ProgramData\chocolatey\lib\NSSM\tools\nssm.exe

We should definitively not install them by default.

@pstorz
Copy link
Member Author

pstorz commented Dec 18, 2024

Hi @pstorz while we are fixing this part, in newer (at least native) build we now have those 2 services installed by default (while they are only needed for testing)

Service Name: bareos_sd-accurate-lmdb-stresstest
Path to Executable: C:\ProgramData\chocolatey\lib\NSSM\tools\nssm.exe

Service Name: bareos_sd-bconsole-basic
Path to Executable: C:\ProgramData\chocolatey\lib\NSSM\tools\nssm.exe

We should definitively not install them by default.

Are you sure that these services are installed by the nsi installer? I would expect that they are setup by the development environment when the sourcecode is tested.

@bruno-at-bareos
Copy link
Contributor

bruno-at-bareos commented Dec 19, 2024

Are you sure that these services are installed by the nsi installer? I would expect that they are setup by the development environment when the sourcecode is tested.

Ok I got caught by this, restarting all tests on a freshly installed Windows 22 doesn't show those services.

@pstorz pstorz changed the title winbareos-native.nsi: do not package python3-fd.dll winbareos-native.nsi: do not package python3 plugins Dec 19, 2024
@bruno-at-bareos bruno-at-bareos self-requested a review December 19, 2024 12:20
bruno-at-bareos
bruno-at-bareos approved these changes Dec 19, 2024
View reviewed changes
Copy link
Contributor

@bruno-at-bareos bruno-at-bareos left a comment

Choose a reason for hiding this comment

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

Thanks for the good work.

@pstorz pstorz force-pushed the dev/pstorz/native-installer-no-python3 branch from 25fb7c1 to a1a203d Compare December 19, 2024 16:56
@pstorz pstorz force-pushed the dev/pstorz/native-installer-no-python3 branch from 7eecef1 to be71f79 Compare December 20, 2024 08:03
@pstorz pstorz mentioned this pull request Dec 20, 2024
Merged
7 tasks
bruno-at-bareos
bruno-at-bareos requested changes Dec 23, 2024
View reviewed changes
Copy link
Contributor

@bruno-at-bareos bruno-at-bareos left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Can we have a look at my comment about keeping the block?

Also please check pr-tool complain about the following changes

Plugin 'cmake format' would modify 'core/CMakeLists.txt'
Plugin 'cmake format' would modify 'core/cmake/BareosGetDistInfo.cmake'
Plugin 'fix copyright year' would modify 'core/cmake/BareosGetDistInfo.cmake'
Plugin 'clang-format check' would modify 'core/src/include/config.h.in'

@bruno-at-bareos
Copy link
Contributor

bruno-at-bareos commented Dec 23, 2024

What I would like to be sure also, is the fact that we don't have check or a trouble by getting those newly quite large signature

status client=bareos-fd

return 124 chars line (which might be even more is client name is already long) bareos-pluton-fd Version: 25.0.0~pre37.777484313 (20 December 2024) VSS Microsoft Windows Server 2012 (build 9200), 64-bit

bruno-at-bareos
bruno-at-bareos approved these changes Dec 31, 2024
View reviewed changes
Copy link
Contributor

@bruno-at-bareos bruno-at-bareos left a comment

Choose a reason for hiding this comment

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

Thanks for all the good work.

@bruno-at-bareos
Copy link
Contributor

bruno-at-bareos commented Dec 31, 2024

Discussed internally:

The uname type in the db is text which is according to postgresql docs unlimited
And in the sql sources we use MAX_ESCAPE_NAME_LENGTH which is 257
So from what I see I do not expect any trouble
In the uname column the client name is even missing:

*sqlquery 
Entering SQL query mode.
Terminate each query with a semicolon.
Terminate query mode with a blank line.
Enter SQL query: select * from client
Add to SQL query: ;
+----------+-----------+---------------------------------------------------------------------------------------+-----------+---------------+--------------+
| clientid | name      | uname                                                                                 | autoprune | fileretention | jobretention |
+----------+-----------+---------------------------------------------------------------------------------------+-----------+---------------+--------------+
|        1 | bareos-fd | 24.0.1~pre10.a2e4f6ebf (22Dec24) Red Hat Enterprise Linux release 8.10 (Ootpa),redhat |         0 |     5,184,000 |   15,552,000 |
|        2 | gonzo-fd  | 24.0.0~pre531.5037b2123 (30Apr24) Fedora release 39 (Thirty Nine),redhat              |         0 |     5,184,000 |   15,552,000 |
+----------+-----------+---------------------------------------------------------------------------------------+-----------+---------------+--------------+

So the uname column in the database has just for the version info without client name 257 chars (unescaped)

@pstorz pstorz force-pushed the dev/pstorz/native-installer-no-python3 branch from 999c216 to 7489a00 Compare December 31, 2024 09:51
@BareosBot BareosBot force-pushed the dev/pstorz/native-installer-no-python3 branch from 06aaae8 to 8544927 Compare December 31, 2024 13:25
@BareosBot BareosBot merged commit d561478 into bareos:master Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bruno-at-bareos bruno-at-bareos bruno-at-bareos approved these changes

Assignees

@pstorz pstorz

@bruno-at-bareos bruno-at-bareos

Labels

requires backport to 24

Projects

None yet

Milestone

25.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@pstorz @bruno-at-bareos @BareosBot