Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Just use a system call to wipe the entire sessions dir #3507

Merged
merged 2 commits into from
Jun 20, 2020

Conversation

PaulWessel
Copy link
Member

No safety issues since this directory was created by GMT. See #3476 for discussion. It does not matter if the user has set their USER_DIR to any area, since we only delete the subdirectory sessions inside that directory. GMT created it, so GMT can delete it. Closes #3476.

No safety issues since this directory was created by GMT.
Copy link
Member

@seisman seisman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good on macOS.

src/clear.c Outdated Show resolved Hide resolved
@PaulWessel
Copy link
Member Author

@joa-quim may wish to check that we are consistently using WIN32 vs _WIN32. I believe _WIN32 is set by the MicroSoft compiler only and hence it is pure Windows, while WIN32 may be set under both Windows and Cygwin. So pure Windows API calls should be _WIN32.

@joa-quim
Copy link
Member

joa-quim commented Jun 20, 2020

Not that obvious. MinGW defines _WIN32 too. The big problem is that we never built/tested MinGW and Cygwin builds. When we want to restrict to VS we should is use _MSC_VER but then it also has versions.

... and we have tons of WIN32

@PaulWessel
Copy link
Member Author

This says things starting with underscore is for compilers providers: https://stackoverflow.com/questions/662084/whats-the-difference-between-the-win32-and-win32-defines-in-c

@joa-quim
Copy link
Member

I was reading that same post

@PaulWessel
Copy link
Member Author

Well, price we pay for supporting a non-Posix OS!
OK to merge this?

@seisman
Copy link
Member

seisman commented Jun 20, 2020

The big problem is that we never built/tested MinGW and Cygwin builds.

Azure Pipepines already have MinGW installed. It seems straightforward to test GMT with MinGW.

@joa-quim
Copy link
Member

... still don't understand what's so f different between them.

@joa-quim
Copy link
Member

Azure Pipepines already have MinGW installed. It seems straightforward to test GMT with MinGW.

That is right, but I really don't feel like start maintaining another build system.

@seisman
Copy link
Member

seisman commented Jun 20, 2020

It seems there are very few people using Cygwin.

@joa-quim
Copy link
Member

Yes, and there even less (0 to my knowledge) BUILDING (try at least) in Cygwin.

@PaulWessel
Copy link
Member Author

still need to get go-ahead to merge though.

@joa-quim
Copy link
Member

You have two (but need none)

@PaulWessel PaulWessel merged commit 1f9c976 into master Jun 20, 2020
@PaulWessel PaulWessel deleted the simplify-clear branch June 20, 2020 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gmt clear fails on Win if session dirs are not empty
3 participants