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

Fix rice-box not found problem #5711

Merged
merged 2 commits into from
Jan 15, 2020
Merged

Conversation

morgo
Copy link
Contributor

@morgo morgo commented Jan 14, 2020

This fixes #5654 after it was discovered that it does not work as intended when the config directory is not present!

Rice boxes do not support being called in an Init function, which means that without refactoring we can currently not use this feature for the init_db.sql file.

Unfortunately this means that the examples need to find a path to init_db.sql. I am not really happy with the solution I went with, but I copied the config/init_db.sql file to examples/local.

The alternative is that we could re-introduce VTROOT as a dependency, or add some sort of symlink in the repo + a copy on making packages. As I said - I'm not happy with the current solution, and really open to ideas.

Thanks @enisoc : it now works as expected.

Edit: This has a second behavior change, which is that the embedded config is no longer rebuilt on make build. This created potentially noisy commits, since it kept bumping the timestamps. It also made the automated build system think it was working on dirty trees.

Signed-off-by: Morgan Tocker tocker@gmail.com

Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

I think it is actually a good thing to have the example be more self-contained and less reliant on files spread out in the code base.

@enisoc
Copy link
Member

enisoc commented Jan 15, 2020

Rice boxes do not support being called in an Init function, which means that without refactoring we can currently not use this feature for the init_db.sql file.

We wouldn't need to call it from an init() function here. This is just a regular function that happens to be called Init(). It gets called explicitly after main() starts, so it should be fine to call rice.FindBox() from inside Mysqld.Init(). We just can't call rice.FindBox() from a package-level variable initializer as we did previously.

Signed-off-by: Morgan Tocker <tocker@gmail.com>
@morgo
Copy link
Contributor Author

morgo commented Jan 15, 2020

We wouldn't need to call it from an init() function here. This is just a regular function that happens to be called Init(). It gets called explicitly after main() starts, so it should be fine to call rice.FindBox() from inside Mysqld.Init(). We just can't call rice.FindBox() from a package-level variable initializer as we did previously.

You are right! it is an Init() function, not an init() function. I've updated it, and it seems to work locally. Let's see if CI works.

I think it is actually a good thing to have the example be more self-contained and less reliant on files spread out in the code base.

That's the goal. I just don't didn't like the implementation.

Signed-off-by: Morgan Tocker <tocker@gmail.com>
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.

3 participants