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.

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

test/feat: Include F2FS root fs in test script #1678

Closed
wants to merge 1 commit into from

Conversation

marfrit
Copy link

@marfrit marfrit commented Apr 6, 2024

I am using F2FS on my Pinebook Pro as the root fs; backintime-cli will not build properly under these circumstances, since the test_tools.py script does not expect a F2FS root. This commit fixes this.

@buhtz
Copy link
Member

buhtz commented Apr 7, 2024

Hello Markus,
Thanks for your contribution.
I don't have any experience with F2FS. I did some Wikpedia research about it. I can not find a answer to the question if it support hardlinks. Do you have any information or reference about it?

Beside this failing tests. Have you used Back In Time on your Pinebook? Does it work? Do you see any indications for problems?

Can you send the output of backintime --diagnostics please?

Kind
Christian

@buhtz buhtz changed the title Include F2FS root fs in test script test/feat: Include F2FS root fs in test script Apr 9, 2024
@buhtz buhtz added the Feedback needs user response, may be closed after timeout without a response label Apr 9, 2024
@buhtz buhtz added this to the Upcoming release (1.5.0) milestone Apr 9, 2024
@buhtz buhtz self-assigned this Apr 9, 2024
@buhtz buhtz marked this pull request as draft April 13, 2024 21:12
@buhtz
Copy link
Member

buhtz commented Apr 13, 2024

I propose to remove the whole test. See my PR #1687 . Will wait for feedback of for my team.

@marfrit
Copy link
Author

marfrit commented Apr 13, 2024

Hi,

documentation I know - https://www.kernel.org/doc/Documentation/filesystems/f2fs.txt - it is designed to be a drop in replacement for ext4 for, e.g., eMMC devices. It supports hardlinks and I haven't had any issues (appart from backintime not installing properly ;)).

@buhtz
Copy link
Member

buhtz commented Apr 14, 2024

Thanks for the feedback.

And please take my apologize that I maybe close this PR without merging. Nevertheless, it's valuable PR as you can see in PR #1687 and three follow-up issues. Your PR triggered some other issues and improved code quality because of that.

@buhtz buhtz removed the Feedback needs user response, may be closed after timeout without a response label Apr 18, 2024
@buhtz buhtz closed this in #1687 Apr 21, 2024
buhtz added a commit that referenced this pull request Apr 21, 2024
Tests remove because of their low value and high maintenance costs.

Close #1678 
Contribution supported by @marfrit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants