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.

Sign up for GitHub

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

Jump to bottom

Fix for finding dependency version in lockfile v2 #446

Merged
merged 1 commit into from
Jan 9, 2020
Merged

Fix for finding dependency version in lockfile v2 #446

merged 1 commit into from
Jan 9, 2020

Conversation

baszalmstra
Copy link
Contributor

With lockfile v2 a version is not required for dependencies if the lock
file only contains a single entry for a specified crate. However, without
a version present, at a later stage, a dependant crate will not be found
in the metadata cache which causes the dependency to be ignored completely.
This commit tries to infer the version of dependant crates when the
version specifier is missing from the contents of the lockfile.

@emilio
Copy link
Collaborator

emilio commented Jan 7, 2020

Thanks for the patch! Could you elaborate on why is this needed?

Also, this needs a test, as it presumably changes behavior.

@baszalmstra
Copy link
Contributor Author

@emilio No problem, I've added the test case that fails without this patch and works with it.

The problem is as following: In the new lock file format, if there is only one version of a dependency then everywhere else in the lockfile the version will be omitted:

# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
[[package]]
name = "dep"
version = "0.1.0"

[[package]]
name = "expand-dep"
version = "0.1.0"
dependencies = [
 "dep",
]

Here the package expand-dep depends on dep but as you might notice not on any particular version. Thats because there is only one version of dep in the lock file anyway.

However, if cbindgen parses this lockfile the parsed version of the dependency dep will be None because it simply cannot parse it (its not there which was already fixed in #438 ). In cargo.rs (fn dependencies) it will also create a PackageRef with a None version. Later, when looking up the dependencies it cannot be found in the metadata because such a package (one without a version) doesn't exist.

This PR patches the building of dependent PackageRefs by building a PackageRef for dependencies that does include the correct version number.

I hope that clears it up. :)

@emilio
Copy link
Collaborator

emilio commented Jan 7, 2020

The test doesn't seem to be passing on automation?

emilio
emilio approved these changes Jan 7, 2020
View reviewed changes
Copy link
Collaborator

@emilio emilio left a comment

Choose a reason for hiding this comment

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

The code looks fine, though we need to make the test pass on CI before merging.

Just one suggestion, if you like it :)

src/bindgen/cargo/cargo.rs Outdated Show resolved Hide resolved
@baszalmstra
Fix for finding dependency version in lockfile v2
ef9c344 
With lockfile v2 a version is not required for dependencies if the lock
file only contains a single entry for a specified crate.  However, without
a version present, at a later stage, a dependant crate will not be found
in the metadata cache which causes the dependency to be ignored completely.
This commit tries to infer the version of dependant crates when the
version specifier is missing from the contents of the lockfile.
@baszalmstra
Copy link
Contributor Author

Thanks @emilio, I've applied your suggestion, fixed the tests and rebased.

emilio
emilio approved these changes Jan 9, 2020
View reviewed changes
Copy link
Collaborator

@emilio emilio left a comment

Choose a reason for hiding this comment

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

Thanks, this looks fine to me :)

@emilio emilio merged commit 8fabbfa into mozilla:master Jan 9, 2020
@baszalmstra
Copy link
Contributor Author

Thanks! Will you create a new release containing this patch?

emilio added a commit that referenced this pull request Jan 10, 2020
@emilio
Release 0.12.2
ac1a7d4 
* Fixed version detection with lockfile v2. #446
* Added support for export_name on functions. #447
@baszalmstra baszalmstra mentioned this pull request Apr 5, 2020
Merged
bors-servo added a commit to servo/servo that referenced this pull request Apr 24, 2020
@bors-servo
Auto merge of #26303 - servo:dependabot/cargo/cbindgen-0.14.1, r=jdm
416de6b 
Bump cbindgen from 0.9.1 to 0.14.1

Bumps [cbindgen](https://github.com/eqrion/cbindgen) from 0.9.1 to 0.14.1.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/eqrion/cbindgen/blob/master/CHANGES">cbindgen's changelog</a>.</em></p>
<blockquote>
<h2>0.14.1</h2>
<pre><code> * Handle mangling pointers. ([#508](mozilla/cbindgen#508))
 * Unconditionally generate a return statement in partialeq implementations. ([#509](mozilla/cbindgen#509))
</code></pre>
<h2>0.14.0</h2>
<pre><code> * Minor tweak at how [export.exclude] is handled to allow excluding
   generic instantiations in C mode. ([#501](mozilla/cbindgen#501))
 * Documented cpp_compat option. ([#496](mozilla/cbindgen#496))
 * Fixed a panic when parsing associated constants for a built-in type. ([#494](mozilla/cbindgen#494))
</code></pre>
<h2>0.13.2</h2>
<pre><code> * Constants now have suitable documentation. ([#471](mozilla/cbindgen#471))
 * Fixed some C warnings by emitting void when there are no arguments. ([#470](mozilla/cbindgen#470))
 * Avoids reading cargo.toml when not needed, which can cause panics in workspace situations.
 * Only write `default` cases if the switch is not exhaustive. ([#475](mozilla/cbindgen#475))
 * Some warnings have been refined. ([#477](mozilla/cbindgen#477))
 * Code generation for static arrays has been fixed. ([#479](mozilla/cbindgen#479))
 * Opt-in support for constexpr in constants. ([#481](mozilla/cbindgen#481))
 * Fix C code generation and some warnings when extremely large constants are used. ([#490](mozilla/cbindgen#490))
 * Proper escaping of enum variants and fields. ([#483](mozilla/cbindgen#483))
 * Added support for RefCell (as an opaque type) and Cell. ([#489](mozilla/cbindgen#489))
</code></pre>
<h2>0.13.1</h2>
<pre><code> * Support `#[cfg]` on individual enum variants. ([#469](mozilla/cbindgen#469))
</code></pre>
<h2>0.13.0</h2>
<pre><code> * Support 'swift_name' attributes on generated functions ([#449](mozilla/cbindgen#449))
 * Add [export.pre_body] to config ([#452](mozilla/cbindgen#452))
 * Handle new line in doc attribute ([#454](mozilla/cbindgen#454))
 * Add support for `Self` in tagged enums, structs and unions ([#455](mozilla/cbindgen#455), [#455](mozilla/cbindgen#455), [#456](mozilla/cbindgen#456))
 * Make sentinel variant respect regular config ([#459](mozilla/cbindgen#459))
 * Fix layout of tagged enums with size under some configurations ([#463](mozilla/cbindgen#463))
 * Add an option to allow configuring the order of function names in generated headers ([#466](mozilla/cbindgen#466))
</code></pre>
<h2>0.12.2</h2>
<pre><code> * Fixed version detection with lockfile v2. mozilla/cbindgen#446
 * Added support for export_name on functions. mozilla/cbindgen#447
</code></pre>
<h2>0.12.1</h2>
<pre><code> * Added support for #[repr*64)] on enums. mozilla/cbindgen#441
 * Added support to generate plain enums instead of enum classes for C++. mozilla/cbindgen#443
 * Fixed dependency resolution with lockfile v2. mozilla/cbindgen#438
</code></pre>
</tr></table> ... (truncated)
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/eqrion/cbindgen/commit/0761b9bbe48d01ded1bbec45bbeea5544b3b1002"><code>0761b9b</code></a> Release 0.14.1</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/33d9ecf11f300bd03f431e9f3417c063e681380c"><code>33d9ecf</code></a> Handle mangling pointers.</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/0fed9eebc47abcd31e0e69357cd39262c8a5a6cc"><code>0fed9ee</code></a> enum: Unconditionally generate a return statement in partialeq implementations.</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/a519f1bda444a0c6c01464296a052d9a05e1d9c4"><code>a519f1b</code></a> Actually use the temp dir for temporary compiled objects.</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/4a38a48937264d6ec162b2fd47ffd0e7a091ca72"><code>4a38a48</code></a> Release 0.14.0</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/39bae60ad7ac4711d8fccc0d098ecef98b37d559"><code>39bae60</code></a> Generate test object files in a temporary directory instead of in-tree</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/ec1631f3e47686e6e0c1b7641bb08b0e39c59250"><code>ec1631f</code></a> Allow excluding monomorph structs in C mode.</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/6fd245096dcd5c50c1065b4bd6ce62a09df0b39b"><code>6fd2450</code></a> Add missing cpp_compat documentation.</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/c265a7562a5ee815057efcc8b1aca31cd3d5318d"><code>c265a75</code></a> parser: Don't panic when finding associated constants to a primitive.</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/17d7aad7d07dce8aa665aedbc75c39953afe1600"><code>17d7aad</code></a> Release v0.13.2</li>
<li>Additional commits viewable in <a href="https://github.com/eqrion/cbindgen/compare/v0.9.1...v0.14.1">compare view</a></li>
</ul>
</details>
<br />

[![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=cbindgen&package-manager=cargo&previous-version=0.9.1&new-version=0.14.1)](https://dependabot.com/compatibility-score/?dependency-name=cbindgen&package-manager=cargo&previous-version=0.9.1&new-version=0.14.1)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language
- `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language
- `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language
- `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language
- `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com):
- Update frequency (including time of day and day of week)
- Pull request limits (per update run and/or open at any time)
- Out-of-range updates (receive only lockfile updates, if desired)
- Security updates (receive only security updates, if desired)

</details>
bors-servo added a commit to servo/servo that referenced this pull request Apr 24, 2020
@bors-servo
Auto merge of #26303 - servo:dependabot/cargo/cbindgen-0.14.1, r=jdm
3355cb3 
Bump cbindgen from 0.9.1 to 0.14.1

Bumps [cbindgen](https://github.com/eqrion/cbindgen) from 0.9.1 to 0.14.1.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/eqrion/cbindgen/blob/master/CHANGES">cbindgen's changelog</a>.</em></p>
<blockquote>
<h2>0.14.1</h2>
<pre><code> * Handle mangling pointers. ([#508](mozilla/cbindgen#508))
 * Unconditionally generate a return statement in partialeq implementations. ([#509](mozilla/cbindgen#509))
</code></pre>
<h2>0.14.0</h2>
<pre><code> * Minor tweak at how [export.exclude] is handled to allow excluding
   generic instantiations in C mode. ([#501](mozilla/cbindgen#501))
 * Documented cpp_compat option. ([#496](mozilla/cbindgen#496))
 * Fixed a panic when parsing associated constants for a built-in type. ([#494](mozilla/cbindgen#494))
</code></pre>
<h2>0.13.2</h2>
<pre><code> * Constants now have suitable documentation. ([#471](mozilla/cbindgen#471))
 * Fixed some C warnings by emitting void when there are no arguments. ([#470](mozilla/cbindgen#470))
 * Avoids reading cargo.toml when not needed, which can cause panics in workspace situations.
 * Only write `default` cases if the switch is not exhaustive. ([#475](mozilla/cbindgen#475))
 * Some warnings have been refined. ([#477](mozilla/cbindgen#477))
 * Code generation for static arrays has been fixed. ([#479](mozilla/cbindgen#479))
 * Opt-in support for constexpr in constants. ([#481](mozilla/cbindgen#481))
 * Fix C code generation and some warnings when extremely large constants are used. ([#490](mozilla/cbindgen#490))
 * Proper escaping of enum variants and fields. ([#483](mozilla/cbindgen#483))
 * Added support for RefCell (as an opaque type) and Cell. ([#489](mozilla/cbindgen#489))
</code></pre>
<h2>0.13.1</h2>
<pre><code> * Support `#[cfg]` on individual enum variants. ([#469](mozilla/cbindgen#469))
</code></pre>
<h2>0.13.0</h2>
<pre><code> * Support 'swift_name' attributes on generated functions ([#449](mozilla/cbindgen#449))
 * Add [export.pre_body] to config ([#452](mozilla/cbindgen#452))
 * Handle new line in doc attribute ([#454](mozilla/cbindgen#454))
 * Add support for `Self` in tagged enums, structs and unions ([#455](mozilla/cbindgen#455), [#455](mozilla/cbindgen#455), [#456](mozilla/cbindgen#456))
 * Make sentinel variant respect regular config ([#459](mozilla/cbindgen#459))
 * Fix layout of tagged enums with size under some configurations ([#463](mozilla/cbindgen#463))
 * Add an option to allow configuring the order of function names in generated headers ([#466](mozilla/cbindgen#466))
</code></pre>
<h2>0.12.2</h2>
<pre><code> * Fixed version detection with lockfile v2. mozilla/cbindgen#446
 * Added support for export_name on functions. mozilla/cbindgen#447
</code></pre>
<h2>0.12.1</h2>
<pre><code> * Added support for #[repr*64)] on enums. mozilla/cbindgen#441
 * Added support to generate plain enums instead of enum classes for C++. mozilla/cbindgen#443
 * Fixed dependency resolution with lockfile v2. mozilla/cbindgen#438
</code></pre>
</tr></table> ... (truncated)
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/eqrion/cbindgen/commit/0761b9bbe48d01ded1bbec45bbeea5544b3b1002"><code>0761b9b</code></a> Release 0.14.1</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/33d9ecf11f300bd03f431e9f3417c063e681380c"><code>33d9ecf</code></a> Handle mangling pointers.</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/0fed9eebc47abcd31e0e69357cd39262c8a5a6cc"><code>0fed9ee</code></a> enum: Unconditionally generate a return statement in partialeq implementations.</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/a519f1bda444a0c6c01464296a052d9a05e1d9c4"><code>a519f1b</code></a> Actually use the temp dir for temporary compiled objects.</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/4a38a48937264d6ec162b2fd47ffd0e7a091ca72"><code>4a38a48</code></a> Release 0.14.0</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/39bae60ad7ac4711d8fccc0d098ecef98b37d559"><code>39bae60</code></a> Generate test object files in a temporary directory instead of in-tree</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/ec1631f3e47686e6e0c1b7641bb08b0e39c59250"><code>ec1631f</code></a> Allow excluding monomorph structs in C mode.</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/6fd245096dcd5c50c1065b4bd6ce62a09df0b39b"><code>6fd2450</code></a> Add missing cpp_compat documentation.</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/c265a7562a5ee815057efcc8b1aca31cd3d5318d"><code>c265a75</code></a> parser: Don't panic when finding associated constants to a primitive.</li>
<li><a href="https://github.com/eqrion/cbindgen/commit/17d7aad7d07dce8aa665aedbc75c39953afe1600"><code>17d7aad</code></a> Release v0.13.2</li>
<li>Additional commits viewable in <a href="https://github.com/eqrion/cbindgen/compare/v0.9.1...v0.14.1">compare view</a></li>
</ul>
</details>
<br />

[![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=cbindgen&package-manager=cargo&previous-version=0.9.1&new-version=0.14.1)](https://dependabot.com/compatibility-score/?dependency-name=cbindgen&package-manager=cargo&previous-version=0.9.1&new-version=0.14.1)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language
- `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language
- `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language
- `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language
- `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com):
- Update frequency (including time of day and day of week)
- Pull request limits (per update run and/or open at any time)
- Out-of-range updates (receive only lockfile updates, if desired)
- Security updates (receive only security updates, if desired)

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilio emilio emilio approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@baszalmstra @emilio