Skip to content

Conversation

mikeland73
Copy link
Contributor

Summary

@Lagoja called out that we were using absolute paths in conf files. Since the conf files are checked-in, they need to be env variables or relative paths if the file doesn't support env variables.

I also added services restart which really helped me test this out.

How was it tested?

devbox add nginx apacheHttpd php
devbox services start
curl localhost
curl localhost:8080
curl localhost:8080/index.php

@mikeland73 mikeland73 requested review from loreto and savil December 16, 2022 05:34
@mikeland73 mikeland73 changed the title [plugins] Use relative paths or env variables in configs [plugins] Use relative paths or env variables in configs DEV-1272 Dec 16, 2022
Copy link
Contributor

@loreto loreto left a comment

Choose a reason for hiding this comment

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

LGTM to get it working properly.

That said, I'm worried that this is easy to mess up by a new plugin writer. Maybe we need to do something like paths are always from the virtenv root and we rewrite them automatically? (so even if you write /foo that becomes {{ .Virtenv }}/foo.

Or something like that; the main point being that it should be hard for plugin writers to get the paths wrong.

@mikeland73
Copy link
Contributor Author

@loreto I agree, this is fragile. I made a bunch of mistakes doing it as well. The challenge is when to do the rewrite, it can't happen on shell in devbox.d because that code is checked-in.

I think this is good for now, but I'll think of ways to make it less error prone.

@mikeland73 mikeland73 merged commit 6fd44c0 into main Dec 16, 2022
@mikeland73 mikeland73 deleted the landau/make-plugin-paths-relative branch December 16, 2022 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants