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

MDEV-18200 MariaBackup - remove symlink requirement from mariabackup.data_directory #2771

Open
wants to merge 1 commit into
base: 10.4
Choose a base branch
from
Open

MDEV-18200 MariaBackup - remove symlink requirement from mariabackup.data_directory #2771

wants to merge 1 commit into from

Conversation

grooverdan
Copy link
Member

  • The Jira issue number for this PR is: MDEV-18200

Description

Test if this runs on Windows.

How can this PR be tested?

Basing the PR against the correct MariaDB version

  • This is a new feature and the PR is based against the latest MariaDB development branch.
  • This is a bug fix and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

  • I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
  • For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

@grooverdan grooverdan mentioned this pull request Sep 25, 2023
Merged
@grooverdan grooverdan force-pushed the bb-10.4-MDEV-18200-mariabackup-assert branch 2 times, most recently from 2c230d1 to 94ebd7d Compare September 26, 2023 00:29
@grooverdan
MDEV-18200 MariaBackup - remove symlink requirement from mariabackup.…
43925ac 
…data_directory

Use move_file instead of chmod for Windows compat.

DATA DIRECTORY as table option not supported under windows so suppress
the warning.
@grooverdan grooverdan force-pushed the bb-10.4-MDEV-18200-mariabackup-assert branch from 94ebd7d to 43925ac Compare September 26, 2023 01:00
@grooverdan grooverdan added the MariaDB Foundation Pull requests created by MariaDB Foundation label Sep 26, 2023
LinuxJedi
LinuxJedi reviewed May 14, 2024
View reviewed changes
Copy link
Contributor

@LinuxJedi LinuxJedi left a comment

Choose a reason for hiding this comment

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

The very purpose of this test is to test the DATA DIRECTORY option. If you don't have symlinks (.isl files) enabled, the DATA DIRECTORY option doesn't work. So, isn't skipping this test correct behaviour? Or am I missing something?

@vaintroub
Copy link
Member

The very purpose of this test is to test the DATA DIRECTORY option. If you don't have symlinks (.isl files) enabled, the DATA DIRECTORY option doesn't work. So, isn't skipping this test correct behaviour? Or am I missing something?

DATA DIRECTORY, with .isl files used to work on Windows without problems. Now it does not, and there is no way to enable it. something somewhere got broken.

@LinuxJedi
Copy link
Contributor

DATA DIRECTORY, with .isl files used to work on Windows without problems. Now it does not, and there is no way to enable it. something somewhere got broken.

Sure, but fudging the test to not test that doesn't seem to be the fix. There should probably be a separate MDEV to fix that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LinuxJedi LinuxJedi LinuxJedi left review comments

@vaintroub vaintroub Awaiting requested review from vaintroub

Assignees
No one assigned
Labels
MariaDB Foundation Pull requests created by MariaDB Foundation
Milestone
No milestone
3 participants
@grooverdan @vaintroub @LinuxJedi