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

Make line ending of generated .md5 compatible with one in the output … #30

Closed
wants to merge 1 commit into from

Conversation

shinji-s
Copy link
Contributor

…of 'cmake -E md5sum' command.

It seems configure_file() by default forces platform specific line ending in the output whereas 'cmake -E md5sum ...' emits LF regardless of the platform. That results in failure of 'cmake -E compare_files' on Windows. I think ultimately cmake should be fixed for consistent behavior but I'm afraid it's too late for that to happen. Without this patch, AppVeyor test for ecmwf/eccodes#41 fails.

@FussyDuck
Copy link

FussyDuck commented Aug 20, 2020

CLA assistant check
All committers have signed the CLA.

@tlmquintino
Copy link
Member

Hi Shinji, thanks for this contribution.
We will bring it in for the next upcoming release 3.4.0 due end of Sep 2020.

tlmquintino
tlmquintino previously approved these changes Sep 7, 2020
Copy link
Member

@tlmquintino tlmquintino left a comment

Choose a reason for hiding this comment

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

This looks like a correct change to ensure consistency in the checks across platforms.

oiffrig
oiffrig previously approved these changes Sep 9, 2020
@oiffrig oiffrig changed the base branch from master to develop September 9, 2020 09:31
@oiffrig oiffrig dismissed stale reviews from tlmquintino and themself September 9, 2020 09:31

The base branch was changed.

@oiffrig
Copy link
Collaborator

oiffrig commented Sep 9, 2020

Merged manually.

@oiffrig oiffrig closed this Sep 9, 2020
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.

4 participants