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

Bug: Windows executable not using the Working Directory #4361

Closed
IndyV opened this issue Feb 16, 2024 · 11 comments
Closed

Bug: Windows executable not using the Working Directory #4361

IndyV opened this issue Feb 16, 2024 · 11 comments

Comments

@IndyV
Copy link

IndyV commented Feb 16, 2024

I wish to call Pocketbase via a script but use a different working directory.

$initialDirectory = Get-Location
$scriptDirectory = Split-Path -Parent $MyInvocation.MyCommand.Path

Start-Process -FilePath "$scriptDirectory\pocketbase.exe" -ArgumentList ($args | ForEach-Object { "$_" }) -WorkingDirectory $initialDirectory

When calling it, it still creates the pb_data folder in $scriptDirectory (where the Pocketbase executable is located).

This also happens when calling it from a nested directory.
I would expect it to create the pb_data folder in the directory I called it from (folderA):

Image

pocketbase_issue

To debug the issue, I made a little executable that gets called the same way as Pocketbase, that shows the variables that get used by Pocketbase (https://github.com/pocketbase/pocketbase/blob/master/pocketbase.go#L256). See the image below:

Image

pocketbase_wd


Expected behavior: I would expect Pocketbase to use the os.Getwd() variable, instead of the filepath.Dir(os.Args[0]).
Talking with others, this seems to behave as expected on OSX .

@ganigeorgiev
Copy link
Member

It is working as intended. If you need to specify a custom pb_data directory use the --dir flag:

./pocketbase serve --dir='/path/to/my/dir'

For more information about the default flag values you can use the --help flag.

@IndyV
Copy link
Author

IndyV commented Feb 16, 2024

While I appreciate the fast response and your time, I'd like to make sure this is working as intended.

Could be that I didn't demonstrate the issue clearly, but the PowerShell documentation shows that -WorkingDirectory specifies the directory the process should start in:

image

The parameter has no effect, so it seems Pocketbase doesn't use os.Getwd() in that case.

Use case:
A (globally installed) npm package which downloads the relative release of Pocketbase. It basically proxies the call to the executable and passes the parameters.
I did try the --dir flag, but I'd need to use "--dir $initialDirectory/pb_data" in the script. I'd like the user to be able to pass in that parameter themselves.

Why?:
https://github.com/pockethost/pbgo#why

Example:

  • PS C:\FolderA> pbgo serve -> Creates pb_data where the npm package is installed, not where it is ran from (FolderA).
  • PS C:\FolderA> pbgo serve --dir ./my_data_folder -> Creates my_data_folder where the npm package is installed, since it handles . as where the executable is located, not where the user called pbgo from (FolderA).

  • Script I'd expect to work (but ignores WorkingDirectory)
     $initialDirectory = Get-Location
     $scriptDirectory = Split-Path -Parent $MyInvocation.MyCommand.Path
     
     try {
       Start-Process -FilePath "$scriptDirectory\pocketbase.exe" -ArgumentList ($args | ForEach-Object { "$_" }) -WorkingDirectory $initialDirectory
     }
     finally {
       Set-Location -Path $initialDirectory
     }
  • (Current script using the --dir flag)


Once again, thank you for your time :)

@ganigeorgiev
Copy link
Member

By default the pb_data is expected to be next to the executable (see also the comment in

// initialize a default data directory based on the executable baseDir
).
You could argue that this is opinionated or that it doesn't work within your expectations but it is not a bug. You are free to provide your own default value if the current one doesn't work for your use case.

@benallfree
Copy link

@ganigeorgiev is this behavior intended to be consistent across platforms? In my tests on OS X, pb_data is created in the current working directory, not the executable directory:

$ mkdir test
$ cd test
$ which pocketbase
/Users/meta/.nvm/versions/node/v20.4.0/bin/pocketbase
$ pocketbase serve
2024/02/16 03:52:56 Server started at http://127.0.0.1:8090
├─ REST API: http://127.0.0.1:8090/api/
└─ Admin UI: http://127.0.0.1:8090/_/
^C%
$ ls
total 0
drwxr-xr-x   3 meta  staff    96B Feb 16 03:52 .
drwxr-xr-x  12 meta  staff   384B Feb 16 03:52 ..
drwxr-xr-x   5 meta  staff   160B Feb 16 03:52 pb_data

@ganigeorgiev
Copy link
Member

You are using an alias and the filepath.Dir(os.Args[0]) I guess will result to your current working directory.

I'll consider replacing os.Args[0] with os.Executable() during the refactoring to try to cover this case.

@benallfree
Copy link

That's interesting, it must be that zsh/bash sends /current/dir/pocketbase as Args[0] even though it resolves it to something in PATH.

If I launch /absolute/path/to/pocketbase serve, it indeed does place pb_data next to the executable.

So the user experience for anyone running pocketbase from a location in PATH, say /usr/bin/pocketbase, is that the baseDir is always cwd.

Not a bug but definitely quirky behavior depending on how pb was launched. I never noticed this with pockethost because we always launch with an explicit --dir.

@ganigeorgiev
Copy link
Member

ganigeorgiev commented Feb 16, 2024

Actually if pocketbase in your example is a symlink I'm not sure that even os.Executable() will produce a consistent cross-platform result, based on the method godoc:

Executable returns the path name for the executable that started the current process. There is no guarantee that the path is still pointing to the correct executable. If a symlink was used to start the process, depending on the operating system, the result might be the symlink or the path it pointed to. If a stable result is needed, path/filepath.EvalSymlinks might help.

Executable returns an absolute path unless an error occurred.

The main use case is finding resources located relative to an executable.

I've added it in my todo to experiment with it but that will be during the refactoring.

In all cases the actual default pb_data path should be always printed in the --help description.

@benallfree
Copy link

Yeah to me it makes the most sense that --dir should default toos.Getwd() for all cases, especially now knowing that os.Args[0] is inconsistent depending on how pocketbase is launched.

@ganigeorgiev
Copy link
Member

ganigeorgiev commented Feb 16, 2024

os.Getwd suffer from similar issue:

If the current directory can be reached via multiple paths (due to symbolic links), Getwd may return any one of them.

Additionally the default PocketBase deployment setup usually fires the executable from within a systemd service file and if we change --dir to default to the cwd user will have to specify also the cwd path in the service configuration which I'd like to avoid.

Having it relative to the executable for me personally is more easy to reason. If we can't find an easy (and performant) way to cover the os.Executable symlink issue mentioned earlier I'm more inclined to add just a note to the cli help description.

@benallfree
Copy link

Yes that makes sense too, especially if it would disrupt legacy installations.

It's very mysterious to me that pb_data is created in cwd without specifying --dir. I haven't been able to track down why that is happening.

@ganigeorgiev
Copy link
Member

It's very mysterious to me that pb_data is created in cwd without specifying --dir. I haven't been able to track down why that is happening.

As mentioned in #4361 (comment), this is most likely because you are accessing the binary indirectly when placed in your PATH (aka. pocketbase serve vs ./pocketbase serve) and filepath.Dir("something") will resolve to . or similar.

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

No branches or pull requests

3 participants