Wrong messages in "Build messages"

[iw2nhl]

Hi,
I noticed that if compilation stop because of a missing file, in "Build messages" you get:
=== Build finished: 0 errors, 0 warnings ===
The same message is shown if you stop the compilation with the "Abort" button.
This is confusing, it should write something like:
=== Build stopped because of errors
and
=== Build aborted by user

If there are missing files, "Build log" shows:
Process terminated with status 1
so this info could be used to change the message in "Build messages".
While for an abort, "Build log" shows:
Process terminated with status 0
but the abort is caused by a user request (button click), so it is a well known condition.

[iw2nhl]

In "plugins\compilergcc\compilergcc.cpp", function "CompilerGCC::OnJobEnd()", starting at line 2974:

Code: cpp

        if (!m_CommandQueue.LastCommandWasRun())
        {
            m_Log->GetTextControl()->SetDefaultStyle(wxTextAttr(COLOUR_NAVY));
            wxString msg = wxString::Format(_("%d errors, %d warnings"), m_Errors.GetCount(cltError), m_Errors.GetCount(cltWarning));
            Manager::Get()->GetMessageManager()->Log(m_PageIndex, msg);
            Manager::Get()->GetMessageManager()->LogToStdOut(msg + _T('\n'));
            LogWarningOrError(cltNormal, 0, wxEmptyString, wxEmptyString, wxString::Format(_("=== Build finished: %s ==="), msg.c_str()));
        }

This is a possible solution for the "file not found" error:

Code: cpp

        if (!m_CommandQueue.LastCommandWasRun())
        {
            m_Log->GetTextControl()->SetDefaultStyle(wxTextAttr(COLOUR_NAVY));
            // --> New code
            int errors = m_Errors.GetCount(cltError);
            int warnings = m_Errors.GetCount(cltWarning);
            if (exitCode != 0 && errors == 0 && warnings == 0)
                wxString msg = wxString::Format(_("errors found"));
            else
                wxString msg = wxString::Format(_("%d errors, %d warnings"), errors, warnings);
            // --> End of new code
            Manager::Get()->GetMessageManager()->Log(m_PageIndex, msg);
            Manager::Get()->GetMessageManager()->LogToStdOut(msg + _T('\n'));
            LogWarningOrError(cltNormal, 0, wxEmptyString, wxEmptyString, wxString::Format(_("=== Build finished: %s ==="), msg.c_str()));
        }

For the "Abort" button I have to investigate more.

[EDIT]
The "Abort" button, calls "CompilerGCC::KillProcess()", same file as before, line 2273; I don't know if something in that function is useful for the "OnJobEnd()" function to know that we are in the "Aborting status". Else we could add a bool for that (both functions are in same class, so it needs only a member variable).

[iw2nhl]

No one interested in this change?
Is there something wrong with it?

[killerbot]

I hadn't seen this post yet. I'll try to have a closer look at it soon. Best thing to do is, provide a patch for this on berlios project page. Changes things get lost in the forum are not that small ;-)

thanks for the help and effort though !!!

[EDIT] : couldn't resist, just checked the first part, looks ok, will test myself first in case of missing file and if ok will adjust the code like you suggested.

[iw2nhl]

Ok, next time I'll add a patch in BerliOS!
Then I have to add a post in the "List of community Patches" thread?

For now, I could post a message to that thread with a link to this thread. (Can I do that?)

[killerbot]

yes you can do that, the patches thread is a nice place for people to see if something already exists without having to go to berlios, and it can show up in forum search results.

[iw2nhl]

I produced that code without knowing wxWidgets very well.
I noticed now that I left a "wxString::Format" that is not required (I just did copy-paste) because there is nothing to insert in the string.
Wrong line

Code: cpp

    wxString msg = wxString::Format(_("errors found"));

It should work, but it is not needed.

[iw2nhl]

It works if the 2 lines that start with "wxString msg" are changed removing "wxString"
"wxString msg" --> "msg"

I noticed now that with latest SVN (and some nigthly ago) it shows correctly "1 error" if an error occurs because a file is not found.
So this patch is no more needed. Someone fixed the code in a better way.

Still remains the "Abort" fix to be done (if wanted).

[killerbot]

It works if the 2 lines that start with "wxString msg" are changed removing "wxString"
"wxString msg" --> "msg"

I noticed now that with latest SVN (and some nigthly ago) it shows correctly "1 error" if an error occurs because a file is not found.
So this patch is no more needed. Someone fixed the code in a better way.

Still remains the "Abort" fix to be done (if wanted).

testing today, but I don't think that part of the code has been changed recently. Your suggested code was indeed incorrect, since you 2 declared in the if/else the local var msg which then died since it was out of scope.

[killerbot]

I just checked and indeed in the build log it says in red : "Process terminated with status 1". So it show some indication of error.
On the other hand maybe we should stick with this, since we are not sure if every return type !=0 is indeed an indication of an error.

[iw2nhl]

I just checked and indeed in the build log it says in red : "Process terminated with status 1". So it show some indication of error.

That message is shown in "Build log", while my patch was for "Build messages", they are different tabs!

On the other hand maybe we should stick with this, since we are not sure if every return type !=0 is indeed an indication of an error.

This rule should be valid for any command-line program, at least in Unix it's like that. Since most of the tools we use (MinGW tools) are ported from linux, this should be a good rule.

[killerbot]

I just checked and indeed in the build log it says in red : "Process terminated with status 1". So it show some indication of error.

That message is shown in "Build log", while my patch was for "Build messages", they are different tabs!

On the other hand maybe we should stick with this, since we are not sure if every return type !=0 is indeed an indication of an error.

This rule should be valid for any command-line program, at least in Unix it's like that. Since most of the tools we use (MinGW tools) are ported from linux, this should be a good rule.

Well : build messages is primarly used for messages of the build which allow you to click on so you end up in the offending code. When I did my test build the focus was on the build log and it was very obvious something was wrong (red line ) ;-)
But then again you said, it was already fixed ... and the patch was no longer needed.

I've learned that cvs can return !=0 although no error : cvs diff

Ok, cvs is not a build command but could be part of an automated thing, let's keep it in mind and think a little bit more about it.

[iw2nhl]

Well : build messages is primarly used for messages of the build which allow you to click on so you end up in the offending code. When I did my test build the focus was on the build log and it was very obvious something was wrong (red line ) ;-)

This was not my case: the focus was on "Build messages" and not on "Build log" and it showed that there were no errors at all. This was a bug.

But then again you said, it was already fixed ... and the patch was no longer needed.

Yes, now it find correctly that there is an error, so it has been fixed.
I don't know how it recognizes the error. Moreover I did not see changes about this in the changelog of last nigthly build... Either it was not listed or it has been a collateral effect of other changes.

I've learned that cvs can return !=0 although no error : cvs diff

Ok, cvs is not a build command but could be part of an automated thing, let's keep it in mind and think a little bit more about it.

Nice, then we have no way to find errors?