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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use proper full install dirs for system config #1580

Merged
merged 1 commit into from Apr 26, 2021

Conversation

dominiklohmann
Copy link
Member

@dominiklohmann dominiklohmann commented Apr 25, 2021

馃摂 Description

Before this change, the default build configuration would try to load schemas from etc/vast/schema instead of /usr/local/etc/vast/schema on macOS, and instead of /etc/vast/schema on Linux systems if VAST_INSTALL_DATADIR was unset or CMAKE_INSTALL_DATADIR set to a relative path instead of an absolute one (it should be relative).

The solution here is to use the full paths instead.

Additionally, there's no need to make the paths configurable, since that ever only affected where VAST looked for the config/schemas/plugins but not where we installed them.

馃摑 Checklist

  • All user-facing changes have changelog entries.
  • The changes are reflected on docs.tenzir.com/vast, if necessary.
  • The PR description contains instructions for the reviewer, if necessary.

馃幆 Review Instructions

File-by-file.

Note that this is based on #1579 to avoid the merge conflict. Other than that the PRs are independent of each other.

@dominiklohmann dominiklohmann added the bug Incorrect behavior label Apr 25, 2021
@dominiklohmann dominiklohmann requested a review from a team April 25, 2021 07:49
@dominiklohmann dominiklohmann force-pushed the topic/install-paths branch 2 times, most recently from c6b7c93 to 8e4a415 Compare April 25, 2021 07:57
@dominiklohmann dominiklohmann marked this pull request as ready for review April 25, 2021 07:57
@dominiklohmann dominiklohmann force-pushed the topic/install-paths branch 2 times, most recently from 5cbd246 to ae4f720 Compare April 25, 2021 19:24
@dominiklohmann dominiklohmann force-pushed the topic/install-paths branch 2 times, most recently from 89325f1 to e58210a Compare April 26, 2021 08:48
Copy link
Member

@tobim tobim left a comment

Choose a reason for hiding this comment

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

Testing rn.

libvast/src/schema.cpp Outdated Show resolved Hide resolved
Before this change, the default build configuration would try to load
schemas from `etc/vast/schema` instead of `/usr/local/etc/vast/schema`
on macOS, and instead of `/etc/vast/schema` on Linux systems if
`VAST_INSTALL_DATADIR` was unset or `CMAKE_INSTALL_DATADIR` set to a
relative path instead of an absolute one (it should be relative).

The solution here is to use the full paths instead.

Additionally, there's no need to make the paths configurable, since that
ever only affected where VAST looked for the config/schemas/plugins but
not where we installed them.
Copy link
Member

@tobim tobim left a comment

Choose a reason for hiding this comment

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

Seems to be working as expected.

Base automatically changed from topic/thread-safe-getenv to master April 26, 2021 09:12
@dominiklohmann dominiklohmann merged commit 2319b19 into master Apr 26, 2021
@dominiklohmann dominiklohmann deleted the topic/install-paths branch April 26, 2021 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior
Projects
None yet
2 participants