From: SourceForge.net <no...@so...> - 2009-07-21 07:59:51
|
Bugs item #2823922, was opened at 2009-07-19 19:24 Message generated for change (Comment added) made by jfaucher You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=684730&aid=2823922&group_id=119701 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Interpreter Group: v4.0beta Status: Open Resolution: None Priority: 5 Private: No Submitted By: jfaucher (jfaucher) Assigned to: jfaucher (jfaucher) Summary: Unix : relative path not supported Initial Comment: This code prints the error message whereas the file exists and the relative path is good : stream = .stream~new("../datas/file.txt") if stream~query("exists") == "" then do say "File "filename" not found" exit 1 end The problems comes from SysFileSystem::qualifyStreamName where the case '.' is not good. If I remove this case to reach the default, then it's ok. I will do the fix, if you agree. I tested with these paths, it's ok : "../datas/file.txt" "../file.txt" "./file.txt" ".file.txt" I don't think I forget a special case for Linux ? Jean-Louis ---------------------------------------------------------------------- Comment By: jfaucher (jfaucher) Date: 2009-07-21 09:59 Message: Mark, Yes, when I tested, I did not think to test with a non existing file :-) The Unix command realpath has the same behavior, error if non existent file. I see that realpath() not only removes "." and ".." but also resolve symbolic links. Do we need to resolve the symbolic links ? I don't see anything written about that in the ooRexx doc. If no, then we can write our own function which removes the "." and "..". Will work even if any part of the path does not exist. And probably safer than using the result of realpath() despite the error code :-) If you agree and not done before next WE, then I will have a look at that. Could be an additional parameter passed to SysFileSystem::canonicalizeName to indicate if realpath() must be used or not. Jean-Louis ---------------------------------------------------------------------- Comment By: Mark Miesfeld (miesfeld) Date: 2009-07-21 07:16 Message: It turns out the behaviour of realpath() is a little quirky, from my point of view. It will canonicalize the path name, but then return NULL and set errno to 2, ENOENT, if the named file does not exist. The print out below shows this. So, for instance, on doing a stream open to write a new file, the interpreter is now failing. Which is why so many test cases are failing. Committed revision 4967. With that commit, the entire test suite runs without any failures on Fedora Core 10 32-bit (trunk.) Not sure if this is the final fix or not, but at least it works with the test suite. <grin> /* qTest.rex */ f = 'streamTest.delMe' ret = stream(f, 'C', "OPEN") say 'ret:' ret '?' (ret == "READY:") [miesfeld@Osprey ooTest]$ rexx qTest.rex Before realpath() name=/home/miesfeld/work.ooRexx/ooTest/rexx.img tempName= After realpath() temp=(nil) name=/home/miesfeld/work.ooRexx/ooTest/rexx.img tempName=/home/miesfeld/work.ooRexx/ooTest/rexx.img Before realpath() name=/home/miesfeld/work.ooRexx/ooTest/rexx.img tempName= After realpath() temp=(nil) name=/home/miesfeld/work.ooRexx/ooTest/rexx.img tempName=/home/miesfeld/work.ooRexx/ooTest/rexx.img Before realpath() name=/home/miesfeld/work.ooRexx/ooTest/framework/rexx.img tempName= After realpath() temp=(nil) name=/home/miesfeld/work.ooRexx/ooTest/framework/rexx.img tempName=/home/miesfeld/work.ooRexx/ooTest/framework/rexx.img Before realpath() name=/opt/slickedit-14.0.1.2/bin/rexx.img tempName= After realpath() temp=(nil) name=/opt/slickedit-14.0.1.2/bin/rexx.img tempName=/opt/slickedit-14.0.1.2/bin/rexx.img Before realpath() name=/tools/dfxRexxLib/rexx.img tempName= After realpath() temp=(nil) name=/tools/dfxRexxLib/rexx.img tempName=/tools Before realpath() name=/usr/lib/qt-3.3/bin/rexx.img tempName= After realpath() temp=(nil) name=/usr/lib/qt-3.3/bin/rexx.img tempName=/usr/lib/qt-3.3/bin/rexx.img Before realpath() name=/usr/kerberos/bin/rexx.img tempName= After realpath() temp=(nil) name=/usr/kerberos/bin/rexx.img tempName=/usr/kerberos/bin/rexx.img Before realpath() name=/usr/lib/ccache/rexx.img tempName= After realpath() temp=(nil) name=/usr/lib/ccache/rexx.img tempName=/usr/lib/ccache/rexx.img Before realpath() name=/usr/local/bin/rexx.img tempName= After realpath() temp=(nil) name=/usr/local/bin/rexx.img tempName=/usr/local/bin/rexx.img Before realpath() name=/usr/bin/rexx.img tempName= After realpath() temp=0xbff546a8 name=/usr/bin/rexx.img tempName=/opt/ooRexx/bin/rexx.img In stream_open() In stream_open() In stream_open() Before realpath() name=/home/miesfeld/work.ooRexx/ooTest/qTest.rex tempName= After realpath() temp=0xbff546e8 name=/home/miesfeld/work.ooRexx/ooTest/qTest.rex tempName=/home/miesfeld/work.ooRexx/ooTest/qTest.rex Before realpath() name=/home/miesfeld/work.ooRexx/ooTest/streamTest.delMe tempName= After realpath() temp=(nil) name=/home/miesfeld/work.ooRexx/ooTest/streamTest.delMe tempName=/home/miesfeld/work.ooRexx/ooTest/streamTest.delMe qualifyStreamName() done=0 tempPath=/home/miesfeld/work.ooRexx/ooTest/streamTest.delMe In stream_open() Before realpath() name=/home/miesfeld/work.ooRexx/ooTest/streamTest.delMe tempName= After realpath() temp=(nil) name=/home/miesfeld/work.ooRexx/ooTest/streamTest.delMe tempName=/home/miesfeld/work.ooRexx/ooTest/streamTest.delMe qualifyStreamName() done=0 tempPath=/home/miesfeld/work.ooRexx/ooTest/streamTest.delMe ret: ERROR:2 ? 0 ---------------------------------------------------------------------- Comment By: Mark Miesfeld (miesfeld) Date: 2009-07-21 02:28 Message: Jean-Louis I added a number of test cases using relative path names for qualify and query exists. On Windows, with trunk, the entire test suite runs without errors. Unfortunately, on Linux the entire test suite has 162 failures and 11 errors. I just started looking at it, but a first guess is that the interpreter is not locating files. For instance, this error: [error] [20090720 17:00:13.511169] svn: r3371 Change date: 2008-09-20 21:33:29 -0700 Test: TEST_RESULT_WITH_RETURN_EXTERNALFILE Class: RESULT_RC_SIGL.testGroup File: /home/miesfeld/.../base/special.variables/RESULT_RC_SIGL.testGroup Event: [SYNTAX 43.1] raised unexpectedly. Could not find routine "ooRexx.Base.RESULT.rex" Line: 199 199 *-* call (fileName) -- call external program comes from this test case: ::method "test_Result_with_Return_ExternalFile" expose fileName call (fileName) -- call external program if var("RESULT") then tmpResult=result -- save value of "result" if var("RC") then tmpRC =rc -- save value of "rc" fileName is set in the test case here: ::method init expose fileName fileName="ooRexx.Base.RESULT.rex" -- define temporary file name forward class (super) So, we'll have to take a closer look at the patch. I glanced through it and thought it seemed okay, but ... ---------------------------------------------------------------------- Comment By: jfaucher (jfaucher) Date: 2009-07-19 23:18 Message: Mark, I committed a new fix, in trunk only. Seems to work as expected. Problem : I won't have time to write test cases now, I will do that probably next WE. I leave you decide when to commit the fix in 4.0.0 (if applicable). Jean-Louis ---------------------------------------------------------------------- Comment By: Mark Miesfeld (miesfeld) Date: 2009-07-19 21:33 Message: Yeah, I thought the impact was wider than it first appeared. One approach would be to write several tests cases to test this aspect throughly, as many as needed. Then fix the code so that they all passed. ---------------------------------------------------------------------- Comment By: jfaucher (jfaucher) Date: 2009-07-19 21:15 Message: It seems we have canonicalizeName which could do that. I will have a look at that... By the way, I see the same kind of problem in SysFileSystem::searchFileName : case '.' That becomes a little bit hot because the impact becomes wider than expected. Jean-Louis ---------------------------------------------------------------------- Comment By: Mark Miesfeld (miesfeld) Date: 2009-07-19 20:20 Message: Jean-Louis, I just checked 3.2.0 on a Linux and my small test program returns: /mnt/local/ooRexx/profiling/csv/stream/datas/file.txt not /mnt/local/ooRexx/profiling/csv/stream/../datas/file.txt So that needs to be fixed also. ---------------------------------------------------------------------- Comment By: Mark Miesfeld (miesfeld) Date: 2009-07-19 19:59 Message: My personal feeling is that f~qualify should return: /mnt/local/ooRexx/profiling/csv/stream/datas/file.txt not /mnt/local/ooRexx/profiling/csv/stream/../datas/file.txt On Windows there are functions to remove the unneeded ..\ from a fully qualified path. Not sure what unix functions there are to do the same thing. But it should be done. You could test your output under 3.2.0 and see what you get. I bet you will get: /mnt/local/ooRexx/profiling/csv/stream/datas/file.txt and if so, then it needs to match that under 4.0.0 -- Mark Miesfeld ---------------------------------------------------------------------- Comment By: jfaucher (jfaucher) Date: 2009-07-19 19:52 Message: Mark, ok, I will commit the fix and add a test case. I have this result for your test : f: /mnt/local/ooRexx/profiling/csv/stream/../datas/file.txt where pwd = /mnt/local/ooRexx/profiling/csv/stream This path is well supported, I can read the file without problem. ---------------------------------------------------------------------- Comment By: Mark Miesfeld (miesfeld) Date: 2009-07-19 19:45 Message: Jean-Louis, Plus, I'd say just go ahead and do the fix. If Rick thinks there is a better way, he's not shy about changing it. ---------------------------------------------------------------------- Comment By: Mark Miesfeld (miesfeld) Date: 2009-07-19 19:40 Message: I don't have a Linux system up right now, but looking at the code, you change seems okay, maybe. <grin> What happens if you do this: f = .stream~new('../datas/file.txt') say 'f:' f~qualify Do you get the right fully qualified path? Can you add a test case that would catch this bug also? -- Mark Miesfeld ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=684730&aid=2823922&group_id=119701 |