Yeah, add -Dos.detected.classifier=osx-x86_64 is a good idea! I have made some tests, we need to add -Dos.detected.classifier=osx-x86_64 or -Dos.detected.classifier=linux-x86_64 in some places when people develop on windows and mac m1 platform using ide.
Take IntelliJ IDEA 2021 as an example: - For maven import: Build, Execution,Deployment - Build Tools - Maven - Importing - VM options for importer. - For maven compile/package with test: Build, Execution,Deployment - Build Tools - Maven - Runner - VM options for importer). This should be also updated on the document where relevant to the project's import and build if we import kudu-binary for test. And I will pay attention to updating the document. About JUnit, there may be a little misunderstanding about it. The purpose of -Dos.detected.classifier=osx-x86_64 / -Dos.detected.classifier=linux-x86_64 is in order to solve the problem when import the project. And the purpose of upgrade about JUnit5 is in order to solve the compatibility about the unit test on windows and mac m1 platform. Consider this situation, we set -Dos.detected.classifier=osx-x86_64 on mac m1 while aarch64 architecture hasn't supported by Kudu, and we will import the kudu-binary-1.12.0-osx-x86_64.jar according to our setting. When we run the unit test or run maven package with test, we will get an error because kudu-binary-1.12.0-osx-x86_64.jar cannot run on mac m1. So I think we need a way to skip the unit test automatically according to the platform. I have found the Assume in JUnit4 can also achieve this goal, so we don't have to upgrade to JUnit5. But I think the annotation in JUnit5 is more elegant, and we don't have too many JUnit4 test so far, so I think this may be a good chance to use JUnit5. Best wishes ! xuankun zheng wenjun <[email protected]> 於 2022年3月17日 週四 下午8:56寫道： > +1 > > Upgrade Kudu from 1.7 to 1.12 looks good to me. As Kirs said, we can > set the property by -D, so we may don't need to upgrade JUnit now. > > Best, > Wenjun Ruan > > On Thu, Mar 17, 2022 at 8:09 PM CalvinKirs <[email protected]> wrote: > > > > Hi, > > +1, That makes sense to me. Thanks for your suggestion. > > but we don't have to explicitly declare the classifier property, when > the user is in an environment like M1, just pass the property with mvn .... > -Dos.detected.classifier=osx-x86_64 . > > > > > > Best wishes！ > > Calvin Kirs > > > > > > On 03/17/2022 01:02，xuankun zheng<[email protected]> wrote： > > Hello everyone, > > > > I want to upgrade the version of Kudu from 1.7 to 1.12 and add some unit > > tests for Kudu according to the previous discussion on github. > > > > More infomation about the discussion can refer to this PR( > > https://github.com/apache/incubator-seatunnel/pull/1459). > > > > And now I want to confirm how about this idea? > > > > Due to Kudu has only supported linux/macOS x86, there may be a little > > problem when we run on Windows or Mac M1 aarch64: > > > > - Maven: We need to set a fixed classifier which is different from the > > Kudu's offcial example. We will get an error when import on Windows/Mac > M1 > > aarch64 platform due to the ${os.detected.classifier} will specify the > > infomation according to our platform, and we can't find the specified > > version of jar. All versions of the jar that Kudu has supported can refer > > to https://repo1.maven.org/maven2/org/apache/kudu/kudu-binary/1.12.0/. > > > > ```xml > > <!-- Windows and Mac M1 aarch64 will get error when import --> > > <dependency> > > <groupId>org.apache.kudu</groupId> > > <artifactId>kudu-binary</artifactId> > > <version>1.12.0</version> > > <classifier>${os.detected.classifier}</classifier> > > <scope>test</scope> > > </dependency> > > > > > > <!-- set a fixed classifier which is linux-x86_64 --> > > <dependency> > > <groupId>org.apache.kudu</groupId> > > <artifactId>kudu-binary</artifactId> > > <version>1.12.0</version> > > <classifier>linux-x86_64</classifier> > > <scope>test</scope> > > </dependency> > > ``` > > > > - Junit: We may need to upgrade the Junit version from Junit4 to Junit5 > in > > order to use the new annotation (@EnabledOnOs, which can make the unit > test > > only run on the specified platform), or we can also use the > > assume(condition) in scalatest to achieve this goal. > > > > So how about the idea which are mentioned above? > > (PS: i am not so good at English. And this is the first time to use this > > form of email, please forgive me if the description is not clear). > > > > Best wishes! >
