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

Introduce SBT_GLOBAL_SERVER_DIR env var to workaround long socket file paths on UNIX #3932

Merged
merged 2 commits into from Feb 13, 2018

Conversation

Projects
None yet
3 participants
@dwijnand
Copy link
Member

commented Feb 8, 2018

Fixes #3930

@dwijnand dwijnand added the in progress label Feb 8, 2018

@dwijnand dwijnand requested a review from cunei Feb 8, 2018

@dwijnand dwijnand added this to the 1.1.2 milestone Feb 8, 2018

@dwijnand dwijnand force-pushed the dwijnand:max-socket-length branch from 9fc00d5 to 8e7dfb4 Feb 8, 2018

case ConnectionType.Local if !Util.isWindows =>
val maxSocketLength = new UnixDomainSocketLibrary.SockaddrUn().sunPath.length - 1
if (socketfile.absolutePath.length > maxSocketLength)
Files.createTempFile("sbt-server", ".sock").toFile // assuming this is short enough..

This comment has been minimized.

Copy link
@cunei

cunei Feb 8, 2018

Contributor

Assuming that there might be multiple independent sbt instances running at the same time, in which case you want the .sock file to be unique, wouldn't it be better to just always hash the string? Something like:

Files.createTempFile("sbt-server-"+socketfile.getCanonicalPath.hashCode.toString, ".sock").toFile

or, even better:

import java.security.MessageDigest

val digest = MessageDigest.getInstance("SHA-256")
d.update(socketfile.getCanonicalPath.getBytes())
Files.createTempFile("sbt-server-"+messageDigest.digest().toString, ".sock").toFile

Please also note that getCanonicalPath is preferable to the absolute path, since the latter is not unique (symbolic links are not resolved.)

This comment has been minimized.

Copy link
@dwijnand

dwijnand Feb 8, 2018

Author Member

Assuming that there might be multiple independent sbt instances running at the same time, in which case you want the .sock file to be unique, wouldn't it be better to just always hash the string?

We do want a unique file - that's what Files.createTempFile does:

 * @return  the path to the newly created file that did not exist before
 *          this method was invoked

This comment has been minimized.

Copy link
@cunei

cunei Feb 8, 2018

Contributor

Right, ok. Then why not just using "sbt-server" as a base, regardless of the dir path?...

This comment has been minimized.

Copy link
@dwijnand

dwijnand Feb 8, 2018

Author Member

Isn't that what I'm doing? Using just "sbt-server" as a base regardless of the dir path?

Files.createTempFile("sbt-server", ".sock").toFile

This comment has been minimized.

Copy link
@dwijnand

dwijnand Feb 8, 2018

Author Member

Do you mean get rid of the ".sock" suffix and use null instead?

This comment has been minimized.

Copy link
@dwijnand

dwijnand Feb 8, 2018

Author Member

I'm not sure what the original design decisions were for having the socket file in the global sbt directory. I'm just leaving that code path as is. Perhaps @eed3si9n can enlighten us.

This comment has been minimized.

Copy link
@eed3si9n

eed3si9n Feb 12, 2018

Member

There are two requirements

  1. sock file should be somewhere the user can secure using chown and chmod. sbt global directory is a good candidate for that.
  2. You can't have two server instances running on a build. That's why h is calculated using the hash of portfile path.

This comment has been minimized.

Copy link
@dwijnand

dwijnand Feb 12, 2018

Author Member

So what do you recommend if the sbt global directory's path is too long to use as the path of the socket?

This comment has been minimized.

Copy link
@eed3si9n

eed3si9n Feb 12, 2018

Member

Maybe introduce a environment variable the user can use to specify "server" directory, and fail to start the server with a message saying either change your global directory or set the env variable?

This comment has been minimized.

Copy link
@dwijnand

dwijnand Feb 12, 2018

Author Member

ok

@dwijnand dwijnand requested a review from eed3si9n Feb 9, 2018

@eed3si9n
Copy link
Member

left a comment

See the requirements I listed in the comment.

case ConnectionType.Local if !Util.isWindows =>
val maxSocketLength = new UnixDomainSocketLibrary.SockaddrUn().sunPath.length - 1
if (socketfile.absolutePath.length > maxSocketLength)
Files.createTempFile("sbt-server", ".sock").toFile // assuming this is short enough..

This comment has been minimized.

Copy link
@eed3si9n

eed3si9n Feb 12, 2018

Member

There are two requirements

  1. sock file should be somewhere the user can secure using chown and chmod. sbt global directory is a good candidate for that.
  2. You can't have two server instances running on a build. That's why h is calculated using the hash of portfile path.

@dwijnand dwijnand force-pushed the dwijnand:max-socket-length branch from fcdf0d5 to 4e038c9 Feb 12, 2018

@dwijnand dwijnand merged commit 8546fc7 into sbt:1.1.x Feb 13, 2018

3 checks passed

Codacy/PR Quality Review Good work! A positive pull request.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@dwijnand dwijnand deleted the dwijnand:max-socket-length branch Feb 13, 2018

@dwijnand dwijnand removed the in progress label Feb 13, 2018

@eed3si9n eed3si9n changed the title Handle very long socket file paths on UNIX Introduce SBT_GLOBAL_SERVER_DIR env var to workaround long socket file paths on UNIX Mar 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.