#1572 TEMP env var is corrupted when running external app

MSYS
pending
None
Bug
rejected
Unknown
False
2013-02-08
2011-08-14
No

On my machine, TEMP environment variable is defined as 'D:\Temp'.

When I run msys, 'echo $TEMP' gives '/tmp', and 'msysmnt' shows that D:\Temp is mounted on /tmp. That's fine.

When I run an external application from msys, the TEMP variable is modified to 'D:/Temp'.

To reproduce: Run msys, from it, run cmd /c "echo %TEMP%"

Discussion

<< < 1 2 (Page 2 of 2)
  • Keith Marshall

    Keith Marshall - 2011-08-18

    Thanks for the patch.

    Attaching it to the ticket would have been infinitely preferable to posting it on pastebin. Never mind: on this occasion only, I've done that for you. Please do it yourself for any future patches you may wish to submit.

    Adding your own ChangeLog entry would also be appreciated. If you wait for us to write it, processing of your patch may be delayed.

     
  • Keith Marshall

    Keith Marshall - 2011-08-18
    • assigned_to: nobody --> cstrauss
    • status: closed-works-for-me --> open
     
  • Orgad Shaneh

    Orgad Shaneh - 2011-08-18

    You're welcome.

    I couldn't attach the file because the issue was closed... Thanks for doing that.

     
  • Keith Marshall

    Keith Marshall - 2011-08-19

    > I couldn't attach the file because the issue was closed...

    I thought the ticket originator could do that, or at least reopen the ticket to progress it. Maybe I'm wrong on that; as administrator, I have more extensive privileges.

    > Thanks for doing that.

    NP. Thanks also for the tentative ChangeLog. For future reference:

    - There's no need to post the whole file; just the entry to add suffices.

    - Your entry should identify each modified file, and function/macro/entity within it, as appropriate; format is defined by the GNU Coding Standards.

    - You may include this within the patch file itself, but NOT as part of the patch proper, (i.e. not in diff format).

    - Normally, the maintainer will adjust the date, to reflect when he applies the patch; thus, it's permissable to fill the date field with query marks.

    I've attached an updated patch, (recreated using 'hg diff -p >> patchfile', where patchfile initially contains the ChangeLog addendum, having applied your original patch to a local mercurial repository copy of the pertinent file, in its correct path from the MSYS CVS), to illustrate the concept.

     
  • Orgad Shaneh

    Orgad Shaneh - 2011-08-19

    Thanks a lot for your help. I really appreciate it.

     
  • Orgad Shaneh

    Orgad Shaneh - 2012-03-14

    ?

     
  • Cesar Strauss

    Cesar Strauss - 2012-03-16

    Sorry, your patch doesn't work for me. After applying it, I still get:

    $ cmd /c "echo %TEMP%"
    C:/Users/cstrauss/AppData/Local/Temp

    Earlier, I intended to investigate this further, but ran out of time.

     
  • Orgad Shaneh

    Orgad Shaneh - 2012-05-16

    Ok, that's really strange. Running cmd /c "echo %TEMP%" indeed prints it with forward slashes, but replacing cmd with cmd.exe does print it with native slashes (with my patch).

    Tried with ActivePerl, it prints forward slashes.

    Any ideas?

     
  • Orgad Shaneh

    Orgad Shaneh - 2012-05-16

    Ok, this patch is still bad. It fixes the TEMP issue, but introduces new problems. Now environment vars that were set during the session are getting a prefix of PWD. grrr...

     
  • Orgad Shaneh

    Orgad Shaneh - 2012-05-16

    Hopefully last try :)

     
  • Orgad Shaneh

    Orgad Shaneh - 2012-05-16

    Looks better now. Please review.

     
  • Earnie Boyd

    Earnie Boyd - 2012-05-16

    I've just looked at the patch. It appears to be barking at the wrong tree. Have you built MSYS in debug mode? If so, strace should give you a value for 'returning: %s'.

     
  • Cesar Strauss

    Cesar Strauss - 2012-06-07

    Your patch interferes with path translation of the command-line. Consider the following program, that prints its arguments:

    #include <stdio.h>

    int main(int argc, char * argv[])
    {
    int i;
    for (i=1; i<argc;i++)
    printf("argv[%d]=%s\n",i,argv[i]);
    return 0;
    }

    Compile it with native (MinGW) gcc, and run on the bash shell:
    ./arg /tmp /c

    Expected result:
    argv[0]=c:\rascunho\msys\home\cstrauss\arg.exe
    argv[1]=C:/Users/cstrauss/AppData/Local/Temp
    argv[2]=/c

    After your patch:
    argv[0]=C:\rascunho\msys\home\cstrauss\arg.exe
    argv[1]=C:\Users\cstrauss\AppData\Local\Temp
    argv[2]=\c

    In particular, the //c -> \c conversion breaks usage of command-line switches, like "cmd //c echo /", which is unacceptable.

    The conversion to backslashes of file paths on the command line (argv[1] above) might be acceptable, however. What the others think?

    As I understand it, the issue on this ticket is due to a double path conversion (native->POSIX->native) losing information about the slashes. When entering MSYS, a few variables are converted to POSIX format, like TEMP and PATH. When invoking a native program, all environment variables are converted back to native format, including TEMP and PATH. However the conversion is always done with forward slashes (except path lists, where backslashes are used). A solution is to always convert to backslashes, but this would affect the command line as well, I think.

    Cesar

     
  • Cesar Strauss

    Cesar Strauss - 2012-06-07

    The test program I used was actually:

    #include <stdio.h>

    int main(int argc, char * argv[])
    {
    int i;
    for (i=0; i<argc;i++)
    printf("argv[%d]=%s\n",i,argv[i]);
    return 0;
    }

     
  • Cesar Strauss

    Cesar Strauss - 2012-06-07

    The test actually was:

    ./arg /tmp //c

    Expected result:
    argv[0]=c:\rascunho\msys\home\cstrauss\arg.exe
    argv[1]=C:/Users/cstrauss/AppData/Local/Temp
    argv[2]=/c

    After your patch:
    argv[0]=C:\rascunho\msys\home\cstrauss\arg.exe
    argv[1]=C:\Users\cstrauss\AppData\Local\Temp
    argv[2]=\c

     
  • Earnie Boyd

    Earnie Boyd - 2012-06-08

    IIRC, most (or all) of that code segment is from the original Cygwin. I don't know if anything will be affected other than perhaps some throughput. PATH was modified to \ only because nothing works if you don't.

     
  • Earnie Boyd

    Earnie Boyd - 2012-10-24
    • milestone: 456516 --> Aged_issue
    • status: open --> open-rejected
     
  • Earnie Boyd

    Earnie Boyd - 2012-10-24

    The patch should only modify the TEMP, TMP and TMPDIR environment variables if they exist just before the spawned native process similar to the PATH variable. It should not modify any item so the resolution is currently rejected. Marking Pending awaiting a proper resolution.

     
  • Earnie Boyd

    Earnie Boyd - 2012-10-24
    • status: open-rejected --> pending-rejected
     
  • Earnie Boyd

    Earnie Boyd - 2013-01-18
    • status: pending-rejected --> pending
    • category: --> Aged_issue
    • milestone: Aged_issue --> MSYS
     
  • Earnie Boyd

    Earnie Boyd - 2013-02-08
    • labels: MSYS -->
    • type: --> Bug
    • resolution: --> rejected
    • category: Aged_issue --> Unknown
    • patch_attached: --> False
     
<< < 1 2 (Page 2 of 2)

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks