Add function for getting the current process path. #1224

jacob-carlborg · 2013-03-25T21:08:35Z

Remove extra space before parentheses in function declaration
Rename to thisProcessPath ?
Move version blocks inside the function ?
Use static assert inside the function instead of regular assert. Needs to be a template or don't declare the function at all.
Don't have empty newline between version blocks
Add missing exception to the docs for Windows
Use a common unittest block with is(typeof(getProcessPath)
Follow @klickverbot's advice to avoid code duplication on Windows

JakobOvrum · 2013-03-25T21:59:13Z

The readlink and Windows versions are missing error handling.

edit:

And wouldn't getExecutablePath be a better name?

BitPuffin · 2013-03-25T22:19:10Z

Huzzah! I can't wait until this gets in (it really should, once minor issues are fixed).

jacob-carlborg · 2013-03-26T07:24:03Z

Hmm, why isn't the comments shown inline?

jacob-carlborg · 2013-03-26T07:48:01Z

The readlink and Windows versions are missing error handling.

Replaced readlink with readLink. On Windows, if it fails, what kind of exception should I throw? Also, should I try to increase the buffer if it's too small? For how long?

And wouldn't getExecutablePath be a better name?

I don't know, what do other thinks?

kyllingstad · 2013-03-26T13:12:12Z

I've replied to the forum post about this. Please consider before merging this.

kyllingstad · 2013-03-26T13:15:49Z

Since this function will always return the same value, should it be cached on the first call?

CyberShadow · 2013-03-26T13:43:03Z

std/process.d

+
+    @trusted string getProcessPath ()
+    {
+        auto buffer = new wchar[MAX_PATH + 1];


Note: this will fail for UNC paths that exceed MAX_PATH. But the same is true for many other functions in Phobos.

You should be allocating a stack buffer here, not a heap buffer that you just throw away. Same goes for all of these actually, better to allocate after you know the length.

@CyberShadow right, I could try and allocate more if needed. But how much and how many times? What exception would be thrown if it fails?

@yebblies I guess it would work for the Windows version. But for Mac OS X, I can't return a slice from a stack allocated buffer?

Right, you'll need to dup it.

@CyberShadow right, I could try and allocate more if needed. But how much and how many times? What exception would be thrown if it fails?

I think it's not the task of this module to worry about memory limits.

From the documentation of GetModuleFileName:

If the buffer is too small to hold the module name, the string is truncated to nSize characters including the terminating null character, the function returns nSize, and the function sets the last error to ERROR_INSUFFICIENT_BUFFER.

So, there you have it. If the first call fails, you'll know exactly how much buffer to allocate.

Erm, sorry, I misread the docs there. It does not return how much you need to allocate.

In cases like this, the usual thing to do is to keep doubling the buffer's length until it's large enough to fit the data.

yebblies · 2013-03-26T14:04:53Z

I needed this the other day, it will be good to have it in phobos.

jacob-carlborg · 2013-03-26T20:33:20Z

Since this function will always return the same value, should it be cached on the first call?

@kyllingstad That might be a good idea.

jacob-carlborg · 2013-03-26T20:59:06Z

Hmm, this commit 19512232c19cf2cdb15d1aecfc145a180bbd13fa didn't show up.

jacob-carlborg · 2013-03-27T07:39:46Z

Windows version will fail until this is merged: dlang/druntime#466

jacob-carlborg · 2013-03-27T07:55:47Z

I failed to find a value for KERN_PROC_PATHNAME.

jacob-carlborg · 2013-03-27T08:00:44Z

Never mind, found it.

jacob-carlborg · 2013-03-27T10:45:54Z

Does anyone have FreeBSD and can help me with its implementation?

JakobOvrum · 2013-03-27T12:05:19Z

std/process.d

+
+    @trusted string getProcessPath ()
+    {
+        static wchar[MAX_PATH + 1] stackBuffer;


This is a TLS buffer, not a stack buffer (which is not necessarily a problem, but the name is wrong).

BitPuffin · 2013-04-10T07:52:53Z

EDIT: Actually turns out that's what you're doing. I suck at github :)

Hmm I read on stackoverflow that the current way you are doing getProcessPath on FreeBSD relies on procfs. What you can do to always have it supported is this:

sysctl CTL_KERN KERN_PROC KERN_PROC_PATHNAME -1

I can't verify that it works though. Maybe the build system can try it out?

Source: http://stackoverflow.com/questions/1023306/finding-current-executables-path-without-proc-self-exe

jacob-carlborg · 2013-04-10T16:46:18Z

Guess I have to install FreeBSD.

yebblies · 2013-06-13T00:00:15Z

std/process.d

+unittest
+{
+    assert(isFile(getProcessPath()));
+}


Please leave a trailing newline at the end of the file.

Why is that?

Some historical stupidity about include files. Reality is some tools complain if you don't have it, so you might as well.

That's almost the most stupid thing I have ever heard of. But ok, I can do that.

http://www.youtube.com/watch?v=97ViuERA2OU

One of those tools is Git itself, by the way, and several Unix-y text editors add it by default.

yebblies · 2013-06-13T00:04:22Z

I would be happy to see this in phobos, even without freebsd support. Forcing users on freebsd to implement this manually would still be better than forcing everyone to implement it manually.

jacob-carlborg · 2013-08-27T12:54:24Z

Maybe this should be in std.path, as it, you know, gets a path. Or in std.file, with getcwd.

As @jpf91 said:

std.path works without file access so it can't do that.

yebblies · 2013-08-27T13:14:32Z

@jacob-carlborg

std.path works without file access so it can't do that.

Most of std.path works without file access. expandTilde obviously does not.

If I was looking for this function (which I was, a couple of months ago) then I would look in std.path, then std.file. std.process does not make a lot of sense. I know it has process in the name, but it has very little to do with the rest of std.process. I think std.file is the best place, as this is closely related to what getcwd does.

ghost · 2013-08-27T13:40:49Z

Argh, what does std.file have to do with the current process path? This fits nicely in std.process.

yebblies · 2013-08-27T13:58:48Z

@AndrejMitrovic

std.process contains "Functions for starting and interacting with other processes, and for working with the current process' execution environment.". This function does not manipulate processes. The word 'process' is less important than grouping related functionality.
std.file contains getcwd, which is closely related to this function. It contains many functions which interact with the filesystem.
std.path is for manipulating paths. Great choice on the grouping functionality front, but it really should be in the same place as getcwd.

getProcessPath has nothing to do with manipulating processes, so I think that std.process is a bad place to put it. It is path and filesystem related, and as I said above it goes well with getcwd in std.file.

I also think we should rename it to currentExecutablePath or something similar (as suggested earlier). The path of the current process is given by getcwd, but this, as the docs say, "Returns the full path of the current executable".

ghost · 2013-08-27T14:11:34Z

The path of the current process is given by getcwd

The path of the working dir of the current process. Anyway I didn't even know getcwd was in std.file, I would assume to find it in std.process. But it's too late to change that.

and for working with the current process' execution environment.".
This function does not manipulate processes.

It's right there in the sentence, working with the current process execution environment.

jacob-carlborg · 2013-08-27T14:32:09Z

I also think we should rename it to currentExecutablePath or something similar (as suggested earlier).

Now it's named thisProcessPath as suggested by @kyllingstad: #1224 (comment)

yebblies · 2013-08-27T14:39:54Z

Now it's named thisProcessPath as suggested by @kyllingstad: #1224

I don't see how this name is any better. A process is a different thing from an executable. This function gives you the name of the file that contains the image used to create the process.

@AndrejMitrovic

The path of the working dir of the current process.

My point is there is potential for confusion with that name. A better name can eliminate this potential. (processExecutablePath?)

This function is closely related to getcwd. It should go with getcwd.

ghost · 2013-08-27T18:33:35Z

I see. Yeah, "process" might be a bad name. I've had a look at some other languages, but they have bizarre ways of getting the path, e.g.:

.net: Process.GetCurrentProcess().MainModule.FileName
Java: getClass().getProtectionDomain().getCodeSource().getLocation().getPath();

Java really takes the cake here for brevity.

Maybe call it getExePath? Well whatever the choice it should be named something short and memorable. As for where to put it, I was bikeshedding a bit, so whatever you guys agree on is ok.

jacob-carlborg · 2013-08-27T18:43:18Z

I've had a look at some other languages, but they have bizarre ways of getting the path

To add a couple of more:

Mac OS X using C API: _NSGetExecutablePath and with D naming scheme getExecutablePath.
Mac OS X using Cocoa API: [[NSBundle mainBundle] executablePath]

I think currentExecutablePath would be the most descriptive name, but it's not very short. Do it really need to be that short? I mean, it's not used in every program. I general I prefer longer more descriptive names to shorter less descriptive names.

ghost · 2013-08-27T18:50:45Z

Does it really need to be that short?

Most functions in Phobos are short, take for example to, text, spawn, which probably have their Java counterparts called convertTypeTo, getObjectAsText, and createThread.

If you have to go for the longer version I'd favor getExecutablePath over current, which is prone to typos (double R), and get is more common and can be easily found.

jacob-carlborg · 2013-08-27T18:55:46Z

I'll change to whatever name you agree on.

ghost · 2013-08-27T18:59:56Z

My vote goes to getExePath, put it next to the declaration of getcwd. @yebblies?

yebblies · 2013-08-28T01:42:54Z

@AndrejMitrovic

getExePath sounds fine. I think it's about as clear as we're going to get. I assume linux people will know what an exe is.

The .net Process.GetCurrentProcess().MainModule.FileName is actually pretty close to how I think of it, which likely shows I've spent too much time with the win32 api.

kyllingstad · 2013-08-28T06:37:28Z

@yebblies, @AndrejMitrovic: Can we please not do the getStuff thing? This is D, not Java. The get prefix does nothing to clarify the purpose nor behaviour of the function; it only makes the name a couple of characters longer. It is also inconsistent with modern Phobos style. I, at least, can't think of a single function which has been added to the library in the past five years which uses this naming convention.

I suggested thisProcessPath for consistency with thisProcessID, which in turn was chosen for symmetry with std.concurrency.thisTid. I'd be happy with thisExe(cutable)Path too, or even just exe(cutable)Path, but please, not get!

I think the function definitely belongs in std.process, as it provides information about the executable from which the process originated. I also think (and have suggested before) that getcwd should be moved to std.process (and renamed!), since it falls into the category of functions which provide information about the process' execution environment. It's not too late, but it should of course follow the normal deprecation procedure.

@jacob-carlborg: Sorry that we are cluttering up your pull request by arguing about this. This has to be the pull request with the greatest discussion/LOC ratio ever. :-)

jacob-carlborg · 2013-08-28T07:37:14Z

@jacob-carlborg: Sorry that we are cluttering up your pull request by arguing about this. This has to be the pull request with the greatest discussion/LOC ratio ever. :-)

168 comments, 169 with this :). As long as you can come to an agreement.

yebblies · 2013-08-28T09:03:44Z

@kyllingstad I'm happy with 'this' instead of 'get'. As I said before this function is about the executable, not the process. If this goes in the same place as getcwd I will be happy enough. Moving getcwd is another argument, and it seems rather pointless to me.

ghost · 2013-08-28T12:49:12Z

Well getcwd is why I thought getExePath would be nice, as it's somewhat consistent, but I don't really mind either way. Honestly we've been bikeshedding so much and giving Jacob an unnecessary amount of work for what should have been a simple pull request.

ghost · 2013-09-04T12:45:09Z

Ok so then would thisExePath next to getcwd be fair? This makes both @yebblies and @kyllingstad happy methinks.

kyllingstad · 2013-09-04T16:31:55Z

@AndrejMitrovic: Semi-happy, but I guess that'll have to do. ;-)

jacob-carlborg · 2013-09-04T16:48:58Z

I would like an answer from @yebblies as well before chaining the name.

yebblies · 2013-09-05T01:30:08Z

@jacob-carlborg I have no problems with thisExePath.

jacob-carlborg · 2013-09-05T12:10:52Z

Then thisExePath it is.

jacob-carlborg · 2013-09-06T09:29:51Z

Renamed to thisExePath and moved to std.file, next to getcwd.

jacob-carlborg · 2013-09-12T16:50:22Z

Any chance this can be pulled so we can leave this behind us?

Add function for getting the current process path.

yebblies · 2013-09-13T04:13:42Z

Thanks for sticking with it. Yay for teamwork.

jacob-carlborg · 2013-09-13T07:35:20Z

Thanks for merging this. I added an enhancement request for this in http://d.puremagic.com/issues/show_bug.cgi?id=11020.

CyberShadow reviewed Mar 26, 2013
View reviewed changes

jacob-carlborg mentioned this pull request Mar 27, 2013

Add Windows error code ERROR_INSUFFICIENT_BUFFER. dlang/druntime#466

Merged

JakobOvrum reviewed Mar 27, 2013
View reviewed changes

yebblies reviewed Jun 13, 2013
View reviewed changes

Add function for getting the current executable path.

bf6c129

yebblies added a commit that referenced this pull request Sep 13, 2013

Merge pull request #1224 from jacob-carlborg/get_process_path

c0d297d

Add function for getting the current process path.

yebblies merged commit c0d297d into dlang:master Sep 13, 2013

jacob-carlborg deleted the get_process_path branch September 13, 2013 07:35

Uh oh!

Add function for getting the current process path. #1224

Add function for getting the current process path. #1224

Uh oh!

Conversation

jacob-carlborg commented Mar 25, 2013

Uh oh!

JakobOvrum commented Mar 25, 2013

Uh oh!

BitPuffin commented Mar 25, 2013

Uh oh!

jacob-carlborg commented Mar 26, 2013

Uh oh!

jacob-carlborg commented Mar 26, 2013

Uh oh!

kyllingstad commented Mar 26, 2013

Uh oh!

kyllingstad commented Mar 26, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yebblies commented Mar 26, 2013

Uh oh!

jacob-carlborg commented Mar 26, 2013

Uh oh!

jacob-carlborg commented Mar 26, 2013

Uh oh!

jacob-carlborg commented Mar 27, 2013

Uh oh!

jacob-carlborg commented Mar 27, 2013

Uh oh!

jacob-carlborg commented Mar 27, 2013

Uh oh!

jacob-carlborg commented Mar 27, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BitPuffin commented Apr 10, 2013

Uh oh!

jacob-carlborg commented Apr 10, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yebblies commented Jun 13, 2013

Uh oh!

jacob-carlborg commented Aug 27, 2013

Uh oh!

yebblies commented Aug 27, 2013

Uh oh!

ghost commented Aug 27, 2013

Uh oh!

yebblies commented Aug 27, 2013

Uh oh!

ghost commented Aug 27, 2013

Uh oh!

jacob-carlborg commented Aug 27, 2013