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

Improve VC++ build log parser #537

Merged
merged 1 commit into from
Jun 28, 2015

Conversation

Bertk
Copy link
Contributor

@Bertk Bertk commented Jun 20, 2015

Tweak VC++ build log parser for visual studio builds logs using normal
verbosity

@jmecosta
Copy link
Member

OK and now I really wany to see the main changes on this. Interested on seeing how you manage to get all info using only normal info

@@ -177,7 +177,7 @@ public void shouldHandleSpeciificV120OptionsCorrectly() {

assertThat(config.getIncludeDirectories().size()).isEqualTo(0);
List<String> defines = config.getDefines();
assertThat(defines.size()).isEqualTo(commonDefines + 8);
Copy link
Member

Choose a reason for hiding this comment

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

why less defines now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved some defines to ParseCommonCompilerOptions() and the compiler version specific part is defined individually e.g.:

    AddMacro("__cplusplus=199711L", fileElement);
    if ... AddMacro("__cplusplus_winrt=201009", fileElement);
    AddMacro("_MSC_VER=1900", fileElement);
    AddMacro("_MSC_FULL_VER=190022816", fileElement);
    AddMacro("_MFC_VER=0x0C00", fileElement);   

The parameter parsing must not be separated as long there is no conflict.
The define for "_M_ARM_FP" is removed (see other comment).

@guwirth guwirth added this to the M 0.9.4 milestone Jun 21, 2015
@guwirth
Copy link
Collaborator

guwirth commented Jun 27, 2015

@Bertk & @jmecosta please tell me when ready to merge.

@Bertk
Copy link
Contributor Author

Bertk commented Jun 28, 2015

@jmecosta Did you have some time to do some tests? I think we are now ready for the merge.

@jmecosta
Copy link
Member

@Bertk i was waiting for the review comments, i will do the tests today.

@jmecosta
Copy link
Member

seems to be running just fine. CR ok

@guwirth
Copy link
Collaborator

guwirth commented Jun 28, 2015

@Bertk could you rebase and squash please

@Bertk
Copy link
Contributor Author

Bertk commented Jun 28, 2015

Sorry. I merged the updates from the master but this should also work for the CI.

@guwirth
Copy link
Collaborator

guwirth commented Jun 28, 2015

@Bertk could you squash (merge commits) please. Much easier to handle in case of problems.

Tweak VC++ build log parser for visual studio builds logs using normal
verbosity

set values for pre-defined macros

Avoid parser errors like "Error evaluating expression '' for AstExp
'IDENTIFIER: _NATIVE_WCHAR_T_DEFINED', assuming 0"

incorporate review comments

Rearrange VS2010 parameter

- remove constant commonDefines
- move VS2010 specific command parameter to ParseV100CompilerOptions()
@Bertk
Copy link
Contributor Author

Bertk commented Jun 28, 2015

I spend some time to make this work "git rebase -i HEAD~4". Notepad++ had a nasty effect which saved the information without any user interaction and I use now notepad to get a chance to modify the instructions - just wasted some time 😑
Should be ready for the merge.

@guwirth
Copy link
Collaborator

guwirth commented Jun 28, 2015

With TortoiseGit it's easy going.

@guwirth
Copy link
Collaborator

guwirth commented Jun 28, 2015

But thx will merge it.

guwirth added a commit that referenced this pull request Jun 28, 2015
@guwirth guwirth merged commit afb7a28 into SonarOpenCommunity:master Jun 28, 2015
@Bertk Bertk deleted the EnhanceVC-BuildLogParser branch July 20, 2015 15:36
@guwirth guwirth mentioned this pull request Oct 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants