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

vector: 0.10.0 -> 0.11.0 #107557

Closed
wants to merge 2 commits into from
Closed

vector: 0.10.0 -> 0.11.0 #107557

wants to merge 2 commits into from

Conversation

happysalada
Copy link
Contributor

Motivation for this change

upgrade vector

Things done

This is more a RFC at this point. @thoughtpolice when you have a moment.

(I've run nixfmt on this, I hope it's not a problem. The only change is version and sha256 corresponding)

Trying to build this, you'll get the

error: failed to sync

Caused by:
  found duplicate version of package `tracing-attributes v0.1.11` vendored from two sources:

        source 1: registry `https://github.com/rust-lang/crates.io-index`
        source 2: https://github.com/tokio-rs/tracing?rev=f470db1b0354b368f62f9ee4d763595d16373231#f470db1b
Traceback (most recent call last):
  File "/nix/store/jfq4wq19ms0m46nkwr9xf3kiv9gqs2jl-cargo-vendor-normalise/bin/.cargo-vendor-normalise-wrapped", line 42, in <module>
    main()
  File "/nix/store/jfq4wq19ms0m46nkwr9xf3kiv9gqs2jl-cargo-vendor-normalise/bin/.cargo-vendor-normalise-wrapped", line 17, in main
    assert list(data.keys()) == ["source"]
AssertionError

This is caused by the fact that vector has a dependency on two libraries named tracing with different sources and versions.
https://github.com/timberio/vector/blob/master/Cargo.lock#L6717
and
https://github.com/timberio/vector/blob/master/Cargo.lock#L6727

I'm not sure what's the best way to resolve this?

In this case, vector has all it's dependencies defined clearly in the cargo.lock, is there even a need to run the vendored script?

@symphorien @Mic92 perhaps you have a better idea of how to solve the problem of having two dependencies named exactly the same?

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

SuperSandro2000
SuperSandro2000 requested changes Dec 25, 2020
View reviewed changes
pkgs/tools/misc/vector/default.nix Outdated Show resolved Hide resolved
pkgs/tools/misc/vector/default.nix
};

cargoSha256 = "Y/vDYXWQ65zZ86vTwP4aCZYCMZuqbz6tpfv4uRkFAzc=";
cargoSha256 = "";
Copy link
Member

Choose a reason for hiding this comment

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

You need to set this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I wanted to make it clear that this needs to be updated. But you are perfectly right!
I'll leave this here until the build issue has been solved.
Thanks!

@kvtb

This comment has been minimized.

Sign in to view
@kvtb kvtb mentioned this pull request Dec 27, 2020
Open
@kvtb

This comment has been minimized.

Sign in to view
@happysalada
Copy link
Contributor Author

Thanks for the comment! I'm guessing this will not be possible to merge in the short term then.
Perhaps I should leave this open for now until something changes.

@symphorien
Copy link
Member

Technically, if you can obtain a "correct" cargo.lock by pinning versions differently, you can use this cargo.lock (see section 15.23.1.5. Building a crate with an absent or out-of-date Cargo.lock file of the nixpkgs manual)

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/comparing-solutions-for-building-rust-on-nix/10707/4

@happysalada
Copy link
Contributor Author

@symphorien Thanks for the update
Here is a link to the rust part of the nixpkgs manual

I've just checked the section, and they suggest using cargo.lock patch. However in this case I'm not even sure what to patch, as the cargo.lock is potentially correct, just referring to two different crates with the same name.
If anybody has an idea of what to patch, I'm open to trying it.

@SuperSandro2000
Copy link
Member

@happysalada please fix the eval error.

@happysalada
Copy link
Contributor Author

@SuperSandro2000 thanks for checking up on this.
I don't know how to fix the eval error. I was hoping for some outside help.
The problem is that there are two packages with the same name and different versions. The cargo vendor step done in nixpkgs doesn't like that.
I don't mind just closing the PR if it's better to not leave it open.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 thanks for checking up on this.
I don't know how to fix the eval error. I was hoping for some outside help.
The problem is that there are two packages with the same name and different versions. The cargo vendor step done in nixpkgs doesn't like that.
I don't mind just closing the PR if it's better to not leave it open.

Oh, didn't read the rest. I would create an issue how upstream wants to support cargo vendor.

@happysalada
Copy link
Contributor Author

the issue was opened upstream. So this is waiting for it. Let me know if anything else is needed.

@Mic92
Copy link
Member

Mic92 commented Jan 12, 2021

I have seen this issue with cargo-vendor before but also don't have a solution for it.

@kvtb
Copy link
Contributor

kvtb commented Jan 31, 2021

I have seen this issue with cargo-vendor before but also don't have a solution for it.

cargo update --aggressive before cargo vendor might help

from vectordotdev/vector#5738 (comment), which is one of the upstream issues triggered by our problem

@symphorien
Copy link
Member

cargo update is not deterministic, as its result depends on network.

@kvtb
Copy link
Contributor

kvtb commented Feb 2, 2021

cargo update is not deterministic, as its result depends on network.

yes, it might result in introduction of another cargoSha256

@thoughtpolice thoughtpolice mentioned this pull request Mar 12, 2021
Merged
10 tasks
@happysalada
Copy link
Contributor Author

superseded

@happysalada happysalada deleted the vector-update branch April 28, 2023 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 requested changes

Assignees
No one assigned
Labels
2.status: merge conflict
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@happysalada @kvtb @symphorien @nixos-discourse @SuperSandro2000 @Mic92