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

[#1487] Fix CAM for Windows, add gitattributes and add Windows CI #1512

Merged
merged 34 commits into from
Mar 4, 2021

Conversation

timo95
Copy link
Contributor

@timo95 timo95 commented Feb 11, 2021

Description

Use \n as line ending in CAM (CanonicalAdjacencyMatrixBuilder). Since the result string isn't meant to be written to files, it is better to keep it the same for all OS's.
Add .gitattributes with options to always checkout text files with LF line endings (regardless of git config). This is needed for the CAM tests in gradoop-flink and gradoop-temporal and for checkstyle (all source code has to be LF).

This restores most tests for Windows builts. Only HBase tests still fail, because to create the HBase mini-cluster on Windows a cygwin environment is necessary (See point 4 of this old HBase doc).

Related Issue

Fixes #1487

Builds on windows work, if HBase tests are skipped. For example by either skipping all tests or by excluding gradoop-hbase.

Edit: I updated the github actions to include Windows and the Readme build instructions to skip tests for Windows.

Motivation and Context

Windows builds

How Has This Been Tested?

I built it on a Windows machine. Tests fail when reaching HBase tests and the build succeeds when skipping tests.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly (if necessary).
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I ran a spell checker.

@timo95 timo95 changed the title [#1478] Fix CAM for windows and add gitattributes [#1487] Fix CAM for windows and add gitattributes Feb 25, 2021
@timo95 timo95 changed the title [#1487] Fix CAM for windows and add gitattributes [#1487] Fix CAM for Windows, add gitattributes and add Windows CI Feb 25, 2021
@timo95
Copy link
Contributor Author

timo95 commented Feb 25, 2021

For the github actions workaround see: actions/runner-images#712

@ChrizZz110
Copy link
Contributor

What are you doing? Please stop ... it's already dead.

@timo95
Copy link
Contributor Author

timo95 commented Feb 26, 2021

Debugging Github Actions. What else?

@timo95
Copy link
Contributor Author

timo95 commented Feb 26, 2021

There seems to be some problem with Hadoops file creation on the Windows VM. It works locally. I will test this with the new version, when #1519 is merged.

@ChrizZz110
Copy link
Contributor

This looks like a lot of fun!

@timo95
Copy link
Contributor Author

timo95 commented Feb 26, 2021

I forgot about winutils. It was already installed on my machine, so it worked there.

@timo95
Copy link
Contributor Author

timo95 commented Feb 26, 2021

The MiniAccumuloCluster seems to fail too, so excluded both Accumulo and HBase.

@ChrizZz110
Copy link
Contributor

The MiniAccumuloCluster seems to fail too, so excluded both Accumulo and HBase.

My test doesn't pass ... ignore it.

@timo95
Copy link
Contributor Author

timo95 commented Feb 26, 2021

The tests are fine^^ Only the test cluster is... not good.

@timo95
Copy link
Contributor Author

timo95 commented Feb 26, 2021

Finally success.

@timo95
Copy link
Contributor Author

timo95 commented Mar 1, 2021

For now i excluded accumulo again. You can review/merge now, @galpha.

I included 2 options to the maven builds to make the log more usable.
--no-transfer-progress removes the dep download progress and -DtrimStackTrace=false shows the whole stack trace on errors.

Copy link
Contributor

@ChrizZz110 ChrizZz110 left a comment

Choose a reason for hiding this comment

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

LGTM - just a minor move of a comment in the readme

README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@galpha galpha left a comment

Choose a reason for hiding this comment

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

LGTM

@galpha galpha merged commit ba108f0 into dbs-leipzig:develop Mar 4, 2021
@timo95 timo95 deleted the 1487_windows branch March 4, 2021 14:55
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.

reintegrate windows build support
3 participants