See if using ; under Windows work better for dirs #1124

PaulWessel · 2019-07-07T19:47:22Z

In movie, we select the directory path separator based on the scripting language, but since this fails on Windows perhaps we should always select ; there. Give it a try on the animation tests, @seisman.

In movie, we select the directory separator based on the scripting language, but since this fails on Windows perhaps we should always select ; there. Give it a try on the animation tests.

seisman · 2019-07-08T02:01:46Z

Still not working. The error message is:

 3/28 Test #150: anim01/anim_01.sh ................***Failed   15.70 sec
Set GMT_SESSION_NAME = 35494
movie_preflight.sh: line 11: D:a1sbuilddocexamplesanim01anim_01: command not found
movie_preflight.sh: line 11: D:/a/1/s/doc/examples/anim01: Is a directory
movie_master.sh: line 12: D:a1sbuilddocexamplesanim01anim_01: command not found
movie_master.sh: line 12: D:/a/1/s/doc/examples/anim01: Is a directory
gmtconvert [ERROR]: Error for input file: No such file (sin_curve.txt)
gmtconvert [ERROR]: Error for input file: No such file (sin_point.txt)
psconvert [ERROR]: Syntax error option -Mb: Cannot read file ..movie_background.ps
end [ERROR]: Failed to call psconvert
end [ERROR]: process_figures returned error 71
mv: cannot stat 'anim_01.ps': No such file or directory
ERROR: gmtest:145
memtrack errors: 0
exit status: 1

PaulWessel · 2019-07-08T02:16:23Z

The first one with any slashes, perhaps there are single backslashes instead? Able to run this through od -c?
I guess the same with ..movie_background.ps - single backslash but executed in bash. I see that one we had before as well. Are you able to manually edit those scripts and paths so that they actually do work? Maybe that will give clues to what we need to change.

seisman · 2019-07-08T03:01:59Z

The content of movie_preflight is:

#!/usr/bin/env bash
# Preflight script
export GMT_SESSION_NAME=$$
source movie_init.sh
gmt math -T0/360/20 T SIND = sin_point.txt
gmt math -T0/360/2 T SIND = sin_curve.txt
gmt begin
# 	Set fixed background output ps name
	gmt figure movie_background ps
	gmt set PS_MEDIA 4ix2i
	gmt set DIR_DATA C:\Users\seisman\Desktop\anim01;C:\Users\seisman\Desktop\anim01\anim_01
	gmt basemap -R0/360/-1.2/1.6 -JX3.5i/1.65i -X0.35i -Y0.25i 	-BWSne+glightgreen -Bxa90g90f30+u'\232' -Bya0.5f0.1g1 --FONT_ANNOT_PRIMARY=9p
gmt end

We can't use ; as the path separator in bash, although it's running on Windows. In bash, ; is used to separator multiple commands. Thus in this script, C:\Users\seisman\Desktop\anim01\anim_01 is interpreted as a command.

PaulWessel · 2019-07-08T03:18:32Z

Hm, so this whole concept of path separation was inherited from how PATH is set in the shell, but in our case DATA_DIR is only parsed internally by GMT - it is never given to any other tool. So having : or ; is kind of arbitrary, except ; caused the problem you mentioned. If we instead had selected another harmless character that is unique and not used in any path or has special meaning to the shell or DOS, then we would be OK. Staring at my keyboard, maybe , ? Would the comma have any unintended consequences, e.g. I assume
gmt set DIR_DATA C:\Users\seisman\Desktop\anim01;C:\Users\seisman\Desktop\anim01\anim_01
is fine, as long as gmt uses the , to split them?
Since we need a backwards-compatible solution (after we verify that this works), perhaps this:

Scan the directory list. If a comma is found, use that as path separator for internal parsing
If not, use the separator for the OS (or in movie, the script language)

What do you think?

seisman · 2019-07-08T03:28:50Z

Good idea to have a platform-independent path separator.

Since DATA_DIR is just for GMT internal use we should not use OS-specific PATH separators but just use a harmless comma.

PaulWessel · 2019-07-08T03:50:16Z

Try again. As soon as we parse DIR_DATA I replace any occurrences of the OS-specific path separator with a comma (hence we are backwards compatible).

seisman · 2019-07-08T05:03:13Z

Still not working. The problem is, this command

gmt set DIR_DATA C:\Users\seisman\Desktop\anim01,C:\Users\seisman\Desktop\anim01\anim_01

is interpreted by bash first. Thus what GMT actually sees is:

gmt set DIR_DATA C:UsersseismanDesktopanim01,C:UsersseismanDesktopanim01anim_01

The following command works as expected.

gmt set DIR_DATA C:/Users/seisman/Desktop/anim01,C:/Users/seisman/Desktop/anim01/anim_01

PaulWessel · 2019-07-08T05:09:07Z

What happened to the environmental parameter that turns off the craziness?

seisman · 2019-07-08T05:15:10Z

No. It's not caused by the MSYS path conversion.

Even on Linux or macOS, you can try

echo gmt set DIR_DATA C:\Users\seisman\Desktop\anim01,C:\Users\seisman\Desktop\anim01\anim_01

and you'll see

gmt set DIR_DATA C:UsersseismanDesktopanim01,C:UsersseismanDesktopanim01anim_01

The reason is that \ is the escape character in Bash.

PaulWessel · 2019-07-08T05:17:02Z

I've seen that using \ (two backslashes) should work. Perhaps we should scan and if we find one we insert a 2nd? Can you test in manually using two works before coding that up?

seisman · 2019-07-08T05:21:45Z

Yes, \\ works for both bash and bat.

PaulWessel · 2019-07-08T05:24:38Z

To be sure, those backslashes are coming from getcw calls in movie, since you're gmt.conf probably has nothing for DATA_DIR when those tests are run. Hence, this is strictly a movie.c issue where we have to double the backslashes when writing out. I think \\ in a printf statement only prints one , so maybe need to insert 3...

PaulWessel · 2019-07-08T05:48:14Z

See if this works then.

PaulWessel · 2019-07-08T06:56:40Z

If it works, please do some command-line experiments with gmt set DIR_DATA to see if we robustly handle ingesting \\ and outputting commas.

joa-quim · 2019-07-08T13:33:23Z

The proposed solution works for the paths but the \\` is needless. Just use /`` for Windows as well.
Next error is

gmt movie main.sh -Sbpre.sh -C4ix2ix125 -Tsin_point.txt  -Nanim_01 -D5 -Mps -Fnone -Q
psconvert [ERROR]: Syntax error option -Mb: Cannot read file ..movie_background.ps

that is again solved replacing

strcat (extra, ",Mb..\\movie_background.ps");

by

strcat (extra, ",Mb../movie_background.ps");

However, next error is: No errors and no movie files. Just damn silence.
Trying the VS debugger is nightmare. I can't step in the system calls.

PaulWessel · 2019-07-08T16:59:38Z

@joa-quim, is a mixed path (mix of forward and backward slashes) OK as well)?

joa-quim · 2019-07-08T17:24:07Z

I believe so.

…

Sent from my iDedo No dia 08/07/2019, às 17:59, Paul Wessel <[email protected]> escreveu:

@joa-quim, is a mixed path (mix of forward and backward slashes) OK as well)? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

PaulWessel · 2019-07-08T17:47:19Z

So to summarize, we should

Use comma-separated lists of path in DATA_DIR and it its documentation [done in this PR].
Always use the forward-slash as directory separator and not check for WIN32 on this issue.

However, when getcwd returns a path with backslashes and we need to write it out (e.g., from movie.c or put_defaults) with fprintf, we MUST either replace all of them with / or insert that second backslash, otherwise we end up writing paths like C:somedirsarenotsepartedfile.txt. Hence,

When directories are obtained from gmt.conf or getcwd, replace \ with / [done in this PR].

hence, the action required is to implement (2) above. Agreed?

seisman · 2019-07-08T18:30:24Z

I think you're right. I just know that Windows supports both / and \ as path separator. Always using slash / seems to be the easiest way.

No longer use backslash in movie for dir separator. Replace any incoming backslash in paths with forward slash.

PaulWessel · 2019-07-08T18:39:44Z

Implemented.

joa-quim · 2019-07-08T19:28:14Z

Maybe so miracle will fix remaining shit. My previous attempt ended up with a file in \anim01\anim_01\master called .... that resists to all attempts to be deleted or seen (permission denied). I'm afraid that I'll have to reboot.

seisman · 2019-07-08T19:53:25Z

This PR may break our testing system. ctest -j 4 now can't run any tests, even on mac.

PaulWessel · 2019-07-08T20:00:02Z

Yikes, what did I do? I added this:

void gmt_replace_backslash_in_path (char *dir) {
	size_t k = 0;
	while (dir[k])
		if (dir[k] == '\\') dir[k] = '/';
}

and call it in gmt_init.c:

gmt_replace_backslash_in_path (GMT->session.DATADIR);

and in movie.c:

gmt_replace_backslash_in_path (datadir); /* Since we will be fprintf the path we must use // for a slash */

joa-quim · 2019-07-08T20:03:50Z

I suspect that if (dir[k] == '\\') dir[k] = '/'; will covert a\\b into a\/b

seisman · 2019-07-08T20:04:59Z

void gmt_replace_backslash_in_path (char *dir) {
	size_t k = 0;
	while (dir[k])
		if (dir[k] == '\\') dir[k] = '/';
}

Do we need a k++ here?

PaulWessel · 2019-07-08T20:06:38Z

Blushing....
Well, yes...

joa-quim · 2019-07-08T20:17:28Z

Ans in fact I have another shit with this. Sometimes it decides to do Portrait others, Landscape. It's not exactly random. It tends to be more Landscape (and thus clipped) on first run and correct (Portrait) on repetitions.

seisman · 2019-07-08T22:39:17Z

After merging this PR, the animation tests all pass on our Windows builds. See #948 if you want to view the build output.

joa-quim · 2019-07-08T22:44:11Z

Hmm, they all still fail for me, though now for a different reason. For example

151/826 Testing: anim02/anim_02.sh
151/826 Test: anim02/anim_02.sh
Command: "C:/MinGW64/msys64/usr/bin/bash.exe" "gmtest" "anim02/anim_02.sh"
Directory: C:/v/build14/doc/examples
"anim02/anim_02.sh" start time: Jul 08 21:29 GMT Daylight Time
Output:
----------------------------------------------------------
Set GMT_SESSION_NAME = 32380
environment: line 3: C:/v/build14/src/gmt: Bad address
grdimage [ERROR]: Failed to read CPT main.cpt.
anim_02.ps: RMS Error = 0.2225 [FAIL]

…s.py (GenericMappingTools#1124) Co-authored-by: Dongdong Tian <[email protected]>

See if using ; under Windows work better for dirs

ba0950a

In movie, we select the directory separator based on the scripting language, but since this fails on Windows perhaps we should always select ; there. Give it a try on the animation tests.

PaulWessel requested a review from seisman July 7, 2019 19:47

Fix typo

d7dc9ac

Use commas for path separators

73cf4f2

Since DATA_DIR is just for GMT internal use we should not use OS-specific PATH separators but just use a harmless comma.

Try the double backslash trick in movie.c

8809948

No more backslashes

ff92f49

No longer use backslash in movie for dir separator. Replace any incoming backslash in paths with forward slash.

joa-quim approved these changes Jul 8, 2019

View reviewed changes

Amazing...

2b4f90f

seisman approved these changes Jul 8, 2019

View reviewed changes

PaulWessel merged commit 1e80dfd into 6.0 Jul 8, 2019

PaulWessel deleted the dosmoviepaths branch July 8, 2019 22:43

seisman mentioned this pull request Jul 11, 2019

See how a file is or is not found #1151

Merged

obaney pushed a commit to obaney/gmt that referenced this pull request Aug 18, 2021

Fix "pandas.Dataframe" to "pandas.DataFrame" in pygmt/datasets/sample…

f012ee6

…s.py (GenericMappingTools#1124) Co-authored-by: Dongdong Tian <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

See if using ; under Windows work better for dirs #1124

See if using ; under Windows work better for dirs #1124

PaulWessel commented Jul 7, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

joa-quim commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

joa-quim commented Jul 8, 2019 via email

PaulWessel commented Jul 8, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

joa-quim commented Jul 8, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

joa-quim commented Jul 8, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

joa-quim commented Jul 8, 2019

seisman commented Jul 8, 2019

joa-quim commented Jul 8, 2019

See if using ; under Windows work better for dirs #1124

See if using ; under Windows work better for dirs #1124

Conversation

PaulWessel commented Jul 7, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

joa-quim commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

joa-quim commented Jul 8, 2019 via email

PaulWessel commented Jul 8, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

joa-quim commented Jul 8, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

joa-quim commented Jul 8, 2019

seisman commented Jul 8, 2019

PaulWessel commented Jul 8, 2019

joa-quim commented Jul 8, 2019

seisman commented Jul 8, 2019

joa-quim commented Jul 8, 2019