Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 10 commits into from
Dec 31, 2024
Merged

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

merged 10 commits into from
Dec 31, 2024

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

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

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'

core/CMakeLists.txt Outdated Show resolved Hide resolved
@bruno-at-bareos
Copy link
Contributor

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

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