Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Force vendoring of LMDB even if a system version is available #699

Merged
merged 2 commits into from
Nov 15, 2022

Conversation

dureuill
Copy link
Contributor

@dureuill dureuill commented Nov 14, 2022

Pull Request

Related issue

Related to meilisearch/meilisearch#3017: will fix once ported to milli and meilisearch.

What does this PR do?

  • Force using vendored version of LMDB
  • don't use lmdb master3 branch anymore: this is a bit of a side effect of using a tag instead of branch for heed as a dependency, but it is wanted anyway for now as lmdb master3 was more of an experiment
  • modifies CI to run cargo check on the release rather than the debug artifacts. This is an attempt to reduce the necessary disk space and avoid "out of space" failures.

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

- don't use lmdb master3 branch anymore
@dureuill dureuill changed the title Force using vendored version of LMDB even if a system version is available Force vendoring of LMDB even if a system version is available Nov 14, 2022
@dureuill dureuill added bug Something isn't working no breaking The related changes are not breaking (DB nor API) labels Nov 14, 2022
Kerollmops
Kerollmops previously approved these changes Nov 14, 2022
Copy link
Member

@Kerollmops Kerollmops left a comment

Choose a reason for hiding this comment

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

I like that. What do you think @irevoire?

irevoire
irevoire previously approved these changes Nov 14, 2022
Copy link
Member

@irevoire irevoire left a comment

Choose a reason for hiding this comment

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

Yep perfect, thanks @dureuill

bors merge

bors bot added a commit that referenced this pull request Nov 14, 2022
699: Force vendoring of LMDB even if a system version is available r=irevoire a=dureuill

# Pull Request

## Related issue
Related to meilisearch/meilisearch#3017: will fix once ported to milli and meilisearch.

## What does this PR do?
- Force using vendored version of LMDB
- **don't use lmdb master3 branch anymore**: this is a bit of a side effect of using a tag instead of branch for heed as a dependency, but it is wanted anyway for now as lmdb master3 was more of an experiment

## PR checklist
Please check if your PR fulfills the following requirements:
- [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
- [x] Have you read the contributing guidelines?
- [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!


Co-authored-by: Louis Dureuil <louis@meilisearch.com>
@bors
Copy link
Contributor

bors bot commented Nov 14, 2022

Build failed:

@irevoire
Copy link
Member

bors merge

bors bot added a commit that referenced this pull request Nov 14, 2022
699: Force vendoring of LMDB even if a system version is available r=irevoire a=dureuill

# Pull Request

## Related issue
Related to meilisearch/meilisearch#3017: will fix once ported to milli and meilisearch.

## What does this PR do?
- Force using vendored version of LMDB
- **don't use lmdb master3 branch anymore**: this is a bit of a side effect of using a tag instead of branch for heed as a dependency, but it is wanted anyway for now as lmdb master3 was more of an experiment

## PR checklist
Please check if your PR fulfills the following requirements:
- [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
- [x] Have you read the contributing guidelines?
- [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!


Co-authored-by: Louis Dureuil <louis@meilisearch.com>
@bors
Copy link
Contributor

bors bot commented Nov 14, 2022

Build failed:

@irevoire
Copy link
Member

bors merge

bors bot added a commit that referenced this pull request Nov 14, 2022
699: Force vendoring of LMDB even if a system version is available r=irevoire a=dureuill

# Pull Request

## Related issue
Related to meilisearch/meilisearch#3017: will fix once ported to milli and meilisearch.

## What does this PR do?
- Force using vendored version of LMDB
- **don't use lmdb master3 branch anymore**: this is a bit of a side effect of using a tag instead of branch for heed as a dependency, but it is wanted anyway for now as lmdb master3 was more of an experiment

## PR checklist
Please check if your PR fulfills the following requirements:
- [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
- [x] Have you read the contributing guidelines?
- [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!


Co-authored-by: Louis Dureuil <louis@meilisearch.com>
@bors
Copy link
Contributor

bors bot commented Nov 14, 2022

Build failed:

@dureuill dureuill dismissed stale reviews from irevoire and Kerollmops via 87576cf November 15, 2022 09:25
@dureuill
Copy link
Contributor Author

The PR now modifies CI to run cargo check on the release rather than the debug artifacts. This is an attempt to reduce the necessary disk space and avoid "out of space" failures.

The idea is to avoid duplicating artifacts between debug and release, and also release artifacts are generally leaner

Copy link
Member

@irevoire irevoire left a comment

Choose a reason for hiding this comment

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

Is this ok for you @Kerollmops?

Copy link
Member

@Kerollmops Kerollmops left a comment

Choose a reason for hiding this comment

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

It is ok for me but @dureuill could you document that in a separate issue because we could probably do better here by probably deleting the target repo or increasing the disk size instead.

bors merge

@bors
Copy link
Contributor

bors bot commented Nov 15, 2022

@dureuill
Copy link
Contributor Author

Opened a new issue like requested: meilisearch/meilisearch#3358

@curquiza curquiza added DB breaking The related changes break the DB and removed no breaking The related changes are not breaking (DB nor API) labels Nov 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working DB breaking The related changes break the DB
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants