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

Darwin (macOS) port for the udffsck tool #1

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

gmerlino
Copy link

@gmerlino gmerlino commented Jan 5, 2018

Hi,

I'm trying to get a workable setup for repairing UDF filesystems under macOS (darwin) as well as Linux and, as discussed in pali#13, possibly merged upstream as well.

With this PR I'm able to get at least the udffsck (not other udftools at the moment) compiled (and launched) without errors. This does not imply I've tested the tool, but as you can verify from the diffs, it is basically standard fixes to the autotools stuff, plus adding some basic header files (taken from other projects verbatim) which are required under macOS but otherwise missing.

To complete the picture, if you (or anyone else) would like to easily test that the PR works as expected, you can use my Homebrew Formula for it:

https://github.com/gmerlino/homebrew-udffsck

At the moment it points to my fork, but I would like to update it accordingly to your repo, if the PR gets merged here, and later on down the road to Pali's repo, once he accepts your udffsck upstream (not to mention, possibly pushing the formula upstream to Homebrew).

Regards,
Giovanni

@argorain
Copy link
Owner

argorain commented Jan 5, 2018

Hi,

thanks for your PR.

I can see your changes are more or less standard fixes, but in fact those changes broke building of udftools project, as you can see here: https://travis-ci.org/argorain/udftools/builds/325280450?utm_source=github_status&utm_medium=notification

udftools is by default for linux and it should compile under linux without any aditional parameter. Therefore it is manadatory to isolate all macOS stuff under some switch or it should compile with default parameters on both platforms.

It would be great if I can run some tests for macOS. Do you know about some service like Travis CI for it?

Thanks,
Vojtech

@gmerlino
Copy link
Author

gmerlino commented Jan 5, 2018

I noticed the failed builds actually, but at first look it's not clear to me the relationship between my modifications and the failures, I'm a bit of a Travis newbie.

I'll give it a deeper look, and try to fix of course.

With regard to macOS in Travis it seems supported actually, see:

https://docs.travis-ci.com/user/multi-os/

@gmerlino
Copy link
Author

gmerlino commented Jan 5, 2018

Fixed building, and thanks for triggering again the Travis tests that previously failed due to transients.

As you may have noticed already, I've also tackled Travis testing under macOS.

This is a quite different PR with respect to the non-fsck-enabled work in: pali#14

I've tried to keep things as separate (and as simple) as possible, because I think it is valuable to have the macOS port merged even before the fsck is fully upstreamed.
Nevertheless I believe this PR is absolutely not invasive, and has it merits whatever the timing (or fate) of the fsck upstreaming.

Please let me know your opinion.

Thanks again for your help.

- ./autogen.sh $TESTS
- ./configure $TESTS
- cd libudffs && make
- cd ../udffsck && make
Copy link
Owner

Choose a reason for hiding this comment

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

Why is testing, x86/x86_64 and gcc disabled for osx? All those tests shall be run.

Keeping osx as allowed failure is good idea, since it is primarily linux package.

#define __BYTE_ORDER BYTE_ORDER
#if __BYTE_ORDER == BIG_ENDIAN
#define __BIG_ENDIAN_BITFIELD
#elif __BYTE_ORDER == LITTLE_ENDIAN
Copy link
Owner

Choose a reason for hiding this comment

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

With respect to pali#14, this needs to be sorted out in same way to prevent future conflict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants