search

fsprojects / FAKE

FAKE - F# Make
https://fake.build
Other
1.28k stars 582 forks source link

On Unix like OSs, look for sqlpackage in directories specified in $PATH #2700

Closed nick-gravgaard closed 1 year ago

nick-gravgaard commented 1 year ago

This PR addresses issue https://github.com/fsprojects/FAKE/issues/2701 of sqlpackage not existing in the same place on MacOS and Linux distros by following the canonical Unix practice of searching the directories specified by the PATH environment variable instead of using a hard-coded path.

mattgallagher92 commented 1 year ago

I've tested this out in my Ubuntu environment, where I have a symlink in /usr/local/bin to my sqlpackage install location, by running the expression Nick has added in F# interactive. It resolves correctly:

    Environment.pathDirectories
    |> Seq.map (fun dir -> !!(dir </> "sqlpackage"))
    |> Seq.concat
    |> Seq.map (fun path -> path, 15)

val it: seq<string * int> = seq [("/usr/local/bin/sqlpackage", 15)]
yazeedobaid commented 1 year ago

Thanks for the PR, I would like to suggest keeping both ways. The old hard-coded way and the commonplace (best practice) as you mentioned. From what I see, now it will search in environment variables only. I'm thinking about a case when a user doesn't have SQL Package in the environment variables but is added to the directory that was hard-coded.

yazeedobaid commented 1 year ago

@mattgallagher92 thanks for testing and confirming the fix

nick-gravgaard commented 1 year ago

Thanks for the feedback @yazeedobaid. I was going to argue that it should be the user's responsibility to ensure their $PATH includes the directory sqlpackage is in, but if we don't do something like you suggest we risk breaking previously working setups. So I agree with you and have pushed up a new commit. First it checks $PATH, falling back to /usr/local/bin (where it currently lives on MacOS), and finally /usr/bin (where it currently lives on the Linux distros I know about).

I think this order is correct. The directories in $PATH should be checked first because that's where users can define their own locations for binaries. The next place should be /usr/local/bin because that's the location for normal user binaries not managed by the distro's package manager, eg. locally compiled packages. Finally we check /usr/bin because that's where distro-managed normal user binaries go.

Having /usr/local/bin before /usr/bin also matches the order in my $PATH as set by my distro, which reinforces my belief this is the most sensible order.