Skip to content

added 32-bit Windows build to GitHub action#3144

Merged
danmar merged 5 commits into
cppcheck-opensource:mainfrom
firewave:win-x86
Feb 24, 2021
Merged

added 32-bit Windows build to GitHub action#3144
danmar merged 5 commits into
cppcheck-opensource:mainfrom
firewave:win-x86

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

No description provided.

…bit Qt binaries are available / rewrite "x86" to "Win32" for solution builds / switched PCRE to NMake for easier build
@firewave firewave marked this pull request as ready for review February 22, 2021 23:10
@firewave firewave changed the title win x86 added Windows x86 build Feb 22, 2021
@firewave firewave changed the title added Windows x86 build added 32-bit Windows build to GitHub action Feb 22, 2021
Copy link
Copy Markdown
Collaborator

@danmar danmar left a comment

Choose a reason for hiding this comment

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

I think this looks good to merge I just have a question.


- name: Run Release test
run: .\bin\testrunner.exe
run: .\bin\testrunner.exe || exit /b !errorlevel!
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what does this exit accomplish? if testrunner succeed why do we need that extra stuff..

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I wasn't sure if the last %ERRORLEVEL% in a batch script is propagated to the caller. I just tested it and if there is just a single command we don't need to do this. Actually we don't need to do it for the last one either but in case someone extends the steps he might miss adding it. So I think it would be fine to keep it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm.. so you are saying that if there are two commands and the first fail and the second succeed.. then this script will succeed. that was unexpected imho. I'd expect an immediate abort.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

A bash script behaves the same unless you specify set -e in it. Batch scripts unfortunately do not have such a "fail all" option.

Copy link
Copy Markdown
Collaborator

@danmar danmar Feb 24, 2021

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It definitely is.
fail-fast has nothing to do with the steps but with the jobs. So in this if the x86 build fails it will immediately cancel the x64 one as well.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm ok.. thanks for the clarification. I am surprised that this CI script might not fail when a step is failing..

@danmar danmar merged commit 77474d0 into cppcheck-opensource:main Feb 24, 2021
@firewave firewave deleted the win-x86 branch February 25, 2021 06:53
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.

2 participants