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

Conflicting build server formatting #2874

Closed
kodebach opened this issue Aug 12, 2019 · 4 comments

Comments

@kodebach
Copy link
Contributor

commented Aug 12, 2019

There is a weird bug in our formatting checks. All build servers use cmake-format version 0.5.4 as far as I can tell.

However for this commit 8801d77 in #2805

When I ran reformat-cmake locally (also cmake-format version 0.5.4, but python3), exactly the files that Cirrus complained about where reformatted. Python 3 is not the cause, since all build servers still use Python 2.7 (see the warnings when calling pip install).

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2019

Thank you for reporting the problem!

Can you also reproduce the other formatting locally? E.g. in different dockers?

Is it maybe related to the OS?

@sanssecours any ideas?

@kodebach

This comment has been minimized.

Copy link
Contributor Author

commented Aug 13, 2019

I doubt it is related to the OS locally I used Linux and none of the Linux servers detected the problem. Also the Cirrus macOS jobs detected the problem, but the Travis macOS job did not.

Since all servers accepted the reformatted version, I guess that the script for testscr_check_formatting does not check all files or doesn't report all faults correctly.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2019

Ok, so it is not like that two different servers came to different formatting results?

To debug the script we could print $files

@sanssecours sanssecours self-assigned this Aug 14, 2019

sanssecours added a commit to sanssecours/elektra that referenced this issue Aug 14, 2019

CMake Format: Fix version detection
The command `cmake-format --version` seems to print the version number
either to `stdout` or `stderr`, depending on the current system. To
make sure we always capture the version number correctly we therefore
redirect `stderr` to the same file descriptor as `stdout`, before we
save the output of the command.

This commit closes ElektraInitiative#2874.

sanssecours added a commit to sanssecours/elektra that referenced this issue Aug 14, 2019

Reformat CMake: Fix version detection
The command `cmake-format --version` seems to print the version number
either to `stdout` or `stderr`, depending on the current system. To
make sure we always capture the version number correctly we therefore
redirect `stderr` to the same file descriptor as `stdout`, before we
save the output of the command.

This commit closes ElektraInitiative#2874.
@sanssecours

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

Ok, so it is not like that two different servers came to different formatting results?

As far as I can tell, cmake-format --version seems to print its output on stderr instead of stdout in the Travis build jobs. The result of this behavior is that the version detection of reformat-cmake fails, and the script does not reformat any files at all. I already fixed the problem in my copy of the repo. If there are no unexpected problems, I will open a pull request soon.

markus2330 added a commit that referenced this issue Aug 18, 2019

Reformat CMake: Fix version detection
The command `cmake-format --version` seems to print the version number
either to `stdout` or `stderr`, depending on the current system. To
make sure we always capture the version number correctly we therefore
redirect `stderr` to the same file descriptor as `stdout`, before we
save the output of the command.

This commit closes #2874.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.