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

JBIDE-21175 - ensure more reliable locating of vagrant command #831

Merged

Conversation

robstryker
Copy link
Member

No description provided.

public static String getVagrantLocation() {
return VAGRANT_LOCATION_LINUX;
return findVagrantLocation();
Copy link
Member

Choose a reason for hiding this comment

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

im wondering if we are better of launching this similar to what @gercan and @ibuziuk been doing for npm and cordova cli tools.

i.e. call out via bash or cmd.exe making it up to the user to have it properly configured in PATH

Copy link
Member Author

Choose a reason for hiding this comment

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

There are times we call out via Runtime.exec() in which case the existing path is in control. But there are also times we use the external tools launch configuration, in which case we need to know the full location. In general, we use the launch config when we want a console to appear with the output, and we use Runtime.exec() when we just want it done in the background.

Copy link
Member Author

Choose a reason for hiding this comment

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

Did some perf-testing on windows and linux. This method is only a little bit slow when it has to go out to which / where, and it will only perform this once per workspace, and only if vagrant isn't found in the path already. Everything's already cached, and it won't try a second time, so the perf. hit is very very very low and the ability to work around path errors is tremendous.

I believe this patch is for the best.

@fbricon
Copy link
Member

fbricon commented Dec 3, 2015

I think looking up the executable through which/%PATH% lookup is acceptable.

It's should prolly be refactored in a utility class though, as we already do the same thing to get the oc binary in

@robstryker robstryker force-pushed the JBIDE-21175_maint branch 3 times, most recently from e123055 to 1d5ded5 Compare December 4, 2015 19:35
Refactored and extracted to cleaner utility method

Further cleanup as per fred
@robstryker robstryker merged commit 4d0f005 into jbosstools:jbosstools-4.3.x Dec 4, 2015
@maxandersen
Copy link
Member

please check with @ibuziuk and @gercan how they do it for npm/etc.

Might be useful here too.

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.

None yet

3 participants