Author Topic: EnvVars Plugin and PATH  (Read 41309 times)

Offline Jenna

  • Administrator
  • Lives here!
  • *****
  • Posts: 7255
Re: EnvVars Plugin and PATH
« Reply #30 on: February 13, 2012, 11:07:38 am »
I guess instead off caching, a better solution would be:
- you read what's currently in the PATH
- you check what needs to be prepended (so it is the folder looked into first)
- if it's not already in the PATH (at front) you put it there in front

So, assume you path is as follows:
C:\LALA
Your projects uses MinGW, so you need to prepend C:\MinGW\bin and C:\Tools\Bin, you end up in:
C:\MinGW\bin;C:\Tools\Bin;C:\LALA
Next time, you use MSVC, so you need to prepend C:\MSVC\bin and C:\WinSDK\Bin you end up in:
C:\MSVC\bin;C:\WinSDK\Bin;C:\MinGW\bin;C:\Tools\Bin;C:\LALA
Next time, you use MinGW again, you end up in:
C:\MinGW\bin;C:\Tools\Bin;C:\MSVC\bin;C:\WinSDK\Bin;C:\MinGW\bin;C:\Tools\Bin;C:\LALA

A clean-up step could remove any obsolete / path's appearing twice. I think there is even a nice wxWidgets class for this, namely wxPathList that helps a lot with the dirty work.

EDIT: One thing to consider for us are macros here btw!

That should be  more or less what is done at the moment, see:
http://svn.berlios.de/wsvn/codeblocks/?op=revision&rev=5344&peg=5344

Needed master path and extra path always are prepended, to come first, doubles are removed.
If the toolchain is not setup correctly, there is nothing we can do in any case.

Actually we should get a path that looks like:

masterpath;extrapaths(s);currentpath_without_masterpath_and_extrapath(s)
Or with your example:

  • C:\LALA
  • C:\MinGW\bin;C:\Tools\Bin;C:\LALA
  • C:\MSVC\bin;C:\WinSDK\Bin;C:\MinGW\bin;C:\Tools\Bin;C:\LALA
  • C:\MinGW\bin;C:\Tools\Bin;C:\MSVC\bin;C:\WinSDK\Bin;C:\LALA

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: EnvVars Plugin and PATH
« Reply #31 on: February 13, 2012, 11:18:53 am »
That should be  more or less what is done at the moment, see:
http://svn.berlios.de/wsvn/codeblocks/?op=revision&rev=5344&peg=5344
Yes, I've seen that already... But the current code is (sorry to say that, but...) a mess. :(

Actually we should get a path that looks like:
masterpath;extrapaths(s);currentpath_without_masterpath_and_extrapath(s)
Working on that... got it soon... will post here... ;-)
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline Jenna

  • Administrator
  • Lives here!
  • *****
  • Posts: 7255
Re: EnvVars Plugin and PATH
« Reply #32 on: February 13, 2012, 11:31:28 am »
That should be  more or less what is done at the moment, see:
http://svn.berlios.de/wsvn/codeblocks/?op=revision&rev=5344&peg=5344
Yes, I've seen that already... But the current code is (sorry to say that, but...) a mess. :(
That's correct, it's grown over some years and was never cleaned up.
So it looks chaotic and has codeparts that are no longer used (like oldpath).

Actually we should get a path that looks like:
masterpath;extrapaths(s);currentpath_without_masterpath_and_extrapath(s)
Working on that... got it soon... will post here... ;-)
It should work exactly like this at the moment, and it should be platform independent due to the use of wxPathList (without the need for us to care about different PATH-specs).

But the main question was/is:
do we really need to cache the systempath.
Can it be harmful not to do it (besides a probably misconfiguration by the user) ?

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: EnvVars Plugin and PATH
« Reply #33 on: February 13, 2012, 11:53:18 am »
But the main question was/is:
do we really need to cache the systempath.
Can it be harmful not to do it (besides a probably misconfiguration by the user) ?
No, we don't - that's the root of the original issue.

OK - I did some clean-up, merged the two methods into one as you proposed. Here is the new code, that should be really bullet-proof:
Code
void CompilerGCC::SetupEnvironment()
{
    if (!CompilerFactory::GetCompiler(m_CompilerId))
        return;

    m_EnvironmentMsg.Clear();

    // look for valid compiler in path
    wxString pathDummy;
    if ( !wxGetEnv(_T("PATH"), &pathDummy) )
    {
        m_EnvironmentMsg = _("Could not read the PATH environment variable!\n"
                             "This can't be good. There may be problems running "
                             "system commands and the application might not behave "
                             "the way it was designed to...");
        return;
    }

    Manager::Get()->GetLogManager()->DebugLog(_T("PATH environment:"));
    Manager::Get()->GetLogManager()->DebugLog(pathDummy);

    const wxString pathSep = wxFileName::GetPathSeparator(); // "\\" or "/"

    // Pre-pend the compiler's master path
    Compiler* compiler = CompilerFactory::GetCompiler(m_CompilerId);
    if (!compiler)
        return;

    wxString masterPath = compiler->GetMasterPath();
    Manager::Get()->GetMacrosManager()->ReplaceMacros(masterPath);
    while (masterPath.Last() == '\\' || masterPath.Last() == '/')
        masterPath.RemoveLast();

    wxString      cApp       = compiler->GetPrograms().C;
    wxArrayString extraPaths = compiler->GetExtraPaths();
    wxString      extraPathsBinPath(wxEmptyString);

    // compile new PATH list:
    wxPathList pathList;
    if ( !masterPath.Trim().IsEmpty() ) // would be very bad, if it *is* empty
    {
        // pre-pend "master path", "master path\bin" and "extra path's"
        pathList.Add(masterPath); // in case there is no "bin" sub-folder
        pathList.Add(masterPath + pathSep + _T("bin"));
    }
    for (unsigned int i=0; i<extraPaths.GetCount(); ++i)
    {
        wxString extraPath = extraPaths[i];
        Manager::Get()->GetMacrosManager()->ReplaceMacros(extraPath);
        while (extraPath.Last() == '\\' || extraPath.Last() == '/')
            extraPath.RemoveLast();
        if (!extraPath.Trim().IsEmpty())
        {
            // Remember, if we found the C application in the extra path's:
            if (   extraPathsBinPath.IsEmpty()
                && wxFileExists(extraPath + pathSep + cApp ) )
                extraPathsBinPath = extraPath;
            pathList.Add(extraPath);
        }
    }
    // append what has already been in the PATH envvar
    pathList.AddEnvList(_T("PATH"));

    bool caseSens = !(platform::windows);

    // try to locate the path to the C compiler:
    // it seems, under Win32, the above command doesn't search in paths with spaces...
    // look directly for the file in question in masterPath
    wxString binPath = pathList.FindAbsoluteValidPath(cApp);
    if (    binPath.IsEmpty()
        || (pathList.Index(wxPathOnly(binPath), caseSens) == wxNOT_FOUND) )
    {
        if      (wxFileExists(masterPath + pathSep + _T("bin") + pathSep + cApp))
            binPath = masterPath + pathSep + _T("bin");
        else if (wxFileExists(masterPath + pathSep + cApp))
            binPath = masterPath;
        else if (!extraPathsBinPath.IsEmpty())
            binPath = extraPathsBinPath;
    }
    else
        binPath = wxPathOnly(binPath);

    // Try again...
    if (    binPath.IsEmpty()
        || (pathList.Index(wxPathOnly(binPath), caseSens)==wxNOT_FOUND) )
    {
        m_EnvironmentMsg << _("Can't find compiler executable in your search path's for ") << compiler->GetName() << _T('\n');
        Manager::Get()->GetLogManager()->DebugLog(F(_T("Can't find compiler executable in your search path's (for %s)..."), compiler->GetName().wx_str()));

        return; // failed to locate compiler executable in path's as provided
    }

    // Clean-up step: Locate duplicate entries and remove them
    wxArrayString envPathArr;
    for (size_t i=0; i<pathList.GetCount(); ++i)
    {
        wxString path = pathList[i].Trim();
        while (path.Last() == '\\' || path.Last() == '/')
            path.RemoveLast();
        if ( !path.IsEmpty() )
        {
            path += pathSep;
            if ( envPathArr.Index(path, caseSens)==wxNOT_FOUND )
                envPathArr.Add(path);
        }
    }

    // Compile the separator used to separate path's in envvars:
    wxString path_app; // used in envvar to separate path's
    if (platform::windows) path_app = _T(";"); else path_app = _T(":");

    // Convert the PATH variable array into a string to apply.
    wxString envPath(binPath + pathSep + path_app); // make sure the bin-path we found is in front
    for (size_t i=0; i<envPathArr.GetCount(); ++i)
    {
        // Skip path to binary we added in front already:
        if ( !envPathArr[i].IsSameAs(binPath + pathSep) )
            envPath += (envPathArr[i] + path_app);
    }

    Manager::Get()->GetLogManager()->DebugLog(_T("Updating compiler PATH environment:"));
    Manager::Get()->GetLogManager()->DebugLog(envPath);
    wxSetEnv(_T("PATH"), envPath);
}

EDIT: For syntax highlighting, see here:
http://pastebin.com/AcXYpNr3

I'll try myself, feel free to inspect and try, too...

Comments?
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: EnvVars Plugin and PATH
« Reply #34 on: February 13, 2012, 11:57:56 am »
Comments?

// Style zealot hat on
Calling Manager::Get()->GetXXXManager() on multiple lines looks wrong and ugly to me
// Style zealot hat off
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: EnvVars Plugin and PATH
« Reply #35 on: February 13, 2012, 12:11:37 pm »
// Style zealot hat on
Calling Manager::Get()->GetXXXManager() on multiple lines looks wrong and ugly to me
// Style zealot hat off
These debug messages will (of-course) be commented later before commit. And then, if you want to see only one of these, you'll need this Manager::Get()->GetXXXManager() stuff... :-)
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: EnvVars Plugin and PATH
« Reply #36 on: February 13, 2012, 12:18:32 pm »
Leaving commented code is event uglier :)
If you want to do that, why don't you define some marco? :)
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: EnvVars Plugin and PATH
« Reply #37 on: February 13, 2012, 12:21:35 pm »
If you want to do that, why don't you define some marco? :)
I recall you have been the one that said macros are ugly. ;-)

Seriously: A macro doesn't help here. Because these debug messages are there to debug a specific function, so IMHO it's easier to leave it commented and un-comment if needed rather then to have a million macros to enable debug messages for a certain/specific function on demand. Might be just my habit though...
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: EnvVars Plugin and PATH
« Reply #38 on: February 13, 2012, 12:39:48 pm »
I recall you have been the one that said macros are ugly. ;-)
In that particular case it was pretty ugly :)

Leaving commented code is pretty ambiguous,
because the reader of the code doesn't know the true intentions of the writer of the code.
Using per file macros is the better way to do it:)
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: EnvVars Plugin and PATH
« Reply #39 on: February 13, 2012, 12:52:53 pm »
Using per file macros is the better way to do it:)
Again: In this case the compiler has many functions and I usually don't want to debug all of them.

Leaving commented code is pretty ambiguous,
because the reader of the code doesn't know the true intentions of the writer of the code.
I think it should be clear by:
Manager::Get()->GetLogManager()->DebugLog( BLAH );

I think there is no generic "always working - always good" way.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline Jenna

  • Administrator
  • Lives here!
  • *****
  • Posts: 7255
Re: EnvVars Plugin and PATH
« Reply #40 on: February 14, 2012, 09:10:18 am »
I'll try myself, feel free to inspect and try, too...

Comments?

Another one to test:
Code
void CompilerGCC::SetupEnvironment()
{
    Compiler* compiler = CompilerFactory::GetCompiler(m_CompilerId);

    if (!compiler)
        return;

    wxString currentPath;
    if ( !wxGetEnv(_T("PATH"), &currentPath) )
    {
        InfoWindow::Display(_("Environment error"),
                            _("Could not read the PATH environment variable!\n"
                              "This can't be good. There may be problems running\n"
                              "system commands and the application might not behave\n"
                              "the way it was designed to..."),
                            15000, 3000);
        return;
    }

    Manager::Get()->GetLogManager()->DebugLogError(_T("PATH environment:"));
    Manager::Get()->GetLogManager()->DebugLogError(currentPath);

    const wxString sep=platform::windows?_T(";"):_T(":");
    const wxString pathSep = wxFileName::GetPathSeparator(); // "\\" or "/"

    wxString      cApp       = compiler->GetPrograms().C;
    wxArrayString extraPaths = compiler->GetExtraPaths();
    wxString      extraPathsBinPath(wxEmptyString);

    // Get configured masterpath, expand macros and remove trailing seperators
    wxString masterPath = compiler->GetMasterPath();
    Manager::Get()->GetMacrosManager()->ReplaceMacros(masterPath);
    while (masterPath.Last() == '\\' || masterPath.Last() == '/')
        masterPath.RemoveLast();

    // Prepend "masterpath/bin" and "masterpath"
    wxPathList pathList;
    if ( !masterPath.Trim().IsEmpty() ) // would be very bad, if it *is* empty
    {
        pathList.Add(masterPath + pathSep + _T("bin"));
        pathList.Add(masterPath); // in case there is no "bin" sub-folder
    }

    // Get configured extrapath(s), expand macros and remove trailing seperators
    for (size_t i=0; i<extraPaths.GetCount(); ++i)
    {
        wxString extraPath = extraPaths[i];
        Manager::Get()->GetMacrosManager()->ReplaceMacros(extraPath);
        while (extraPath.Last() == '\\' || extraPath.Last() == '/')
            extraPath.RemoveLast();
        if (!extraPath.Trim().IsEmpty())
        {
            // Remember, if we found the C application in the extra path's:
            if ( extraPathsBinPath.IsEmpty()
                && wxFileExists(extraPath + pathSep + cApp ) )
                    extraPathsBinPath = extraPath;
            pathList.Add(extraPath);
        }
    }


    // append what has already been in the PATH envvar
    // if we do it this way, paths are automatically normalized
    // and doubles are removed
    wxPathList pathArray;
    pathArray.AddEnvList(_T("PATH"));

    pathList.Add(pathArray);

    bool caseSense = !(platform::windows);

    // try to locate the path to the C compiler:
    wxString binPath = pathList.FindAbsoluteValidPath(cApp);

    // it seems, under Win32, the above command doesn't search in paths with spaces...
    // look directly for the file in question in masterPath if it is not already found
    if (binPath.IsEmpty()
        || (pathList.Index(wxPathOnly(binPath), caseSense) == wxNOT_FOUND) )
    {
        if (wxFileExists(masterPath + pathSep + _T("bin") + pathSep + cApp))
            binPath = masterPath + pathSep + _T("bin");
        else if (wxFileExists(masterPath + pathSep + cApp))
            binPath = masterPath;
        else if (!extraPathsBinPath.IsEmpty())
            binPath = extraPathsBinPath;
    }
    else
        binPath = wxPathOnly(binPath);

/* TODO (jens#1#): Is the above correct ?
Or should we search in the whole systempath (pathList in this case) for the executable ?*/
    // Try again...
    if ( binPath.IsEmpty() ||
        (pathList.Index(wxPathOnly(binPath), caseSense) == wxNOT_FOUND) )
    {
        InfoWindow::Display(_("Environment error"),
                            _("Can't find compiler executable in your configured search path's for ") + compiler->GetName() + _T('\n'));
        Manager::Get()->GetLogManager()->DebugLogError(F(_T("Can't find compiler executable in your configured search path's (for %s)..."), compiler->GetName().wx_str()));

        return; // failed to locate compiler executable in path's as provided
    }

    // Convert the pathList into a string to apply.
    wxString envPath(binPath); // make sure the bin-path we found is in front
    // and remove it from pathList
    pathList.Remove(binPath);

    for (size_t i=0; i<pathList.GetCount(); ++i)
    {
            envPath += (sep + pathList[i] );
    }

    Manager::Get()->GetLogManager()->DebugLogError(_T("Updating compiler PATH environment:"));
    Manager::Get()->GetLogManager()->DebugLogError(envPath);
    wxSetEnv(_T("PATH"), envPath);
}

I mades some changes, they are mostly commented in the sources.
It avoids the trailing path-seperators in the path(s)  and the trailing seperator at the end of the system-path value.
I use the ability of wxPathList to avoid doubles and to normalize the paths, to avoid redundance.
I do not use m_EnvironmentMsg, because it isonly used for generated makefiles, which are not supported since ages.

It's tested on linux, but not (yet) on windows, seems to work as expected.

By the way (and off-topic here):
I think we should remove all the makefile generation stuff from our sources, because it is not used anymore.
We have a working (as far as I know) tool from the community to create makefiles from project files, that works (again as far as I know) much better and can surely be wrapped by (or possibly be converted to) a plugin to fully integrate it into C::B.

EDIT:


I think it's safer to use:
Code
    wxPathList pathArray;
    pathArray.AddEnvList(_T("PATH"));

instead of

Code
    wxArrayString pathArray = GetArrayFromString(currentPath, sep);

I changed it in the codesnippet above.
« Last Edit: February 14, 2012, 10:18:25 am by jens »

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: EnvVars Plugin and PATH
« Reply #41 on: February 14, 2012, 10:46:10 am »
Another one to test:
Looks fine to me, too.

By the way (and off-topic here):
I think we should remove all the makefile generation stuff from our sources, because it is not used anymore.
Certainly tru. And (in fact) I did this already in my local copy literally since ages. I have a rather cleaned-up compiler plugin basically ready to commit for that purpose (that's also, why I can hardly provide true patches). So if there are no objections, I could step-by-step commit the changes.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: EnvVars Plugin and PATH
« Reply #42 on: February 14, 2012, 11:13:04 am »
Concerning this:
Code
    /* TODO (jens#1#): Is the above correct ?
       Or should we search in the whole systempath (pathList in this case) for the executable? */
Yes, I think its correct. We should enforce that the user does a correct setup of the compiler. Otherwise we don't know, if we picked the right compiler if we search the whole system path. We may even find a wrong one, probably for another platform (avr) which we pick then as "correct". This can only go wrong and we will have a lot of trouble question in the forums hard to track.

But what might be wrong is the fact, tat we only check for the C compiler. For a C++ project this might not be needed. And for a Makefile project not, too.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: EnvVars Plugin and PATH
« Reply #43 on: April 07, 2013, 06:50:51 am »
Is this issue fixed in the trunk?

To solve the problem I describe here: Does Compiler plugin automatically add lib search path to PATH environment, I need to modify PATH variable in the EnvVars plugin by setting the PATH variable to "%PATH%;E:\code\opencv\OpenCV-2.4.4\opencv\build\x86\mingw\bin;E:\code\gcc\nixman463dw2\bin", just the steps described in pkrcel's original post EnvVars Plugin and PATH, it works OK (C::B rev 8896)

I'm not clear how compiler plugin modify PATH, in my case, my compiler Tool Chain executable path is: E:\code\gcc\PCXMinGW463, but as the opencv library is prebuild, it need a libstdc++-6.dll, so I add the E:\code\gcc\nixman463dw2\bin to the PATH. So, I can guess that when compiling, I have two gcc paths. Does the compiler plugin always put the "Tool Chain executable path" path in the front of the PATH? I mean, the executables under "E:\code\gcc\PCXMinGW463" will be chosen to build my app?

If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.