CompilerXML::AutoDetectInstallationDir() quirk

[AndrewCot]

If the masterpath is already set and the code calls CompilerXML::AutoDetectInstallationDir()  then the code goes through a bunch of checks and ends up with the following check:

Code Select Expand

    if (   wxFileExists(m_MasterPath + wxFILE_SEP_PATH + wxT("bin") + wxFILE_SEP_PATH + m_Programs.C)
        || wxFileExists(m_MasterPath + wxFILE_SEP_PATH + m_Programs.C)
        || (GetID() == wxT("null")) ) // Special case so "No Compiler" is valid
    {
        return adrDetected;
    }

This check is not performed in entry to the function, but the following block is executed after the variables are setup/configured

Code Select Expand

    if (!m_MasterPath.IsEmpty())
    {
        path += wxPATH_SEP + m_MasterPath;
        wxSetEnv(wxT("PATH"), path);
        m_MasterPath.Clear();
    }

The block above (if (!m_MasterPath.IsEmpty())) has two potential issues:

   
• It adds the m_MasterPath to the end of the path.  I thought it would be better to add it to the front. Have I missed something?
• The 'return adrDetected;" block could be added into the first code block above (removing the GetID()..) as follows so the detection is quicker. Again have I missed something?

The final code would look like the following:

Code Select Expand

    if (!m_MasterPath.IsEmpty())    {        if (wxFileExists(m_MasterPath + wxFILE_SEP_PATH + wxT("bin") + wxFILE_SEP_PATH + m_Programs.C) || wxFileExists(m_MasterPath + wxFILE_SEP_PATH + m_Programs.C))        {
            return adrDetected;
        }

        path = m_MasterPath + wxPATH_SEP + path;
        wxSetEnv(wxT("PATH"), path);
        m_MasterPath.Clear();
    }


[Miguel Gimenez]

Path management is feeble, to say the least, but fixing it seems to break tons of existing projects.

[AndrewCot]

Can you elaborate how the path change will break tons of projects as the path change is reverted in the function before any returns? If you have an example of something that fails or works differently or could work differently please let me know as I do not want to break anything or cause any issues as it seems odd that the masterpath is added to the end of the path so when the code seaches fro the compiler later it could find a different compiler on the path if one exists before the existing one (I got hit with this yesterday in my testing of clang on Windows MSYS2 as the clang exe can be in mingw32, mingw64, clang32 and clang64 and...).


Does the additional check and early exit make sense? Have I missed anything with this?