search

darsto / brother-scand

Network driver for Brother scanner devices. Daemon for press-to-scan functionality.
MIT License
7 stars 16 forks source link

Minimum changes to build on clang-11.0.3 (clang-1103.0.32.62) #9

Open chickenandpork opened 3 years ago

chickenandpork commented 3 years ago

Minimum changes to build on clang-11.0.3 (clang-1103.0.32.62) for x86_64-apple-darwin19.5.0

NOTE: I'm not a fan of cmake-ing the world, my personal experience hasn't lead to great cmake support experiences, I'd rather bazelify-the-world personally or drop back to autotools for most compatibility :) For this reason, I see @rumpeltux's https://github.com/darsto/brother-scand/pull/6 but would prefer autotools or bazel to cmake.

To avoid resistance based on replacing the build tool, I've cut a smaller PR to simply update this repo to the minimum required to build.

Apple's Clang-11.0.3 is:

Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/4.2.1
Apple clang version 11.0.3 (clang-1103.0.32.62)
Target: x86_64-apple-darwin19.5.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
rumpeltux commented 3 years ago

Hi, would you mind basing this on top of my https://github.com/rumpeltux/brother-scand version of #6 to avoid diverging this further? (Unfortunately this @darsto’s repo seems dead…)

In my branch, CMake is not required for building, just for running the unit tests. Regular building continues to use plain Makefiles.

Also maybe you can extend the README for Apple build instructions, otherwise it seems like the commands from this PR’s message would be hard to find for future users.

chickenandpork commented 3 years ago

I might base it on a branch that has zero requirement for cmake if that helps you.

I don’t use cmake, so I cannot test anything that uses cmake, so I cannot test a PR based on something that needs cmake unfortunately.

chickenandpork commented 3 years ago

I do agree that fragmentation splits effort across the board: 10 engineers working on one project get further than 10 working on 3. We all move forward that way

rumpeltux commented 3 years ago

The unittests use googletest which only supports cmake / bazel IIRC. Feel free to add a bazel build file as well. The rest of the functionality doesn’t need cmake at all.

The other branches don’t have any tests anyways.

darsto commented 3 years ago

I'm surprised to see any activity here. I'm obviously not maintaining this repo anymore so I'm probably not going to merge any pull requests, but I can add a note to the main readme saying that development is continued on @rumpeltux fork

@rumpeltux do you want me to do that?

rumpeltux commented 3 years ago

I'm surprised to see any activity here. I'm obviously not maintaining this repo anymore so I'm probably not going to merge any pull requests, but I can add a note to the main readme saying that development is continued on @rumpeltux fork

@rumpeltux do you want me to do that?

Thank you, yes, that’d be great. I’m happy to address incoming pull requests.

darsto commented 3 years ago

@rumpeltux Thank you, done. I'm going to archive this repository now, which should make it read-only.