Try   HackMD

Cargo team meeting notes 2024 Q3

2024-09-24

  • ed: FCP reminders
  • eric: msrv stabilization
    • ed: local branch ready, still working on documentation, set up in parallel discussions with josh, will wait for last night's MSRV change for people to test that. Still planning this release.
    • josh: Got through 90% discussion at rustconf, very close.
  • weihang: default to lockfile v4
    • Stabilized in https://github.com/rust-lang/cargo/pull/12852
    • Has been stabilized since 1.78.0 (about 8 months)
    • ed: lockfile tied to msrv in workspace, so if that works we can potentially be more aggressive to stabilizing
    • No objections, weihang will submit PR to stabilize it.
  • josh: cfg(true) and cfg(false) - https://github.com/rust-lang/rfcs/pull/3695
    • should cargo implement this as well? Lower rationale, no obvious use case, but would be good for consistency.
    • ed: the more differences, the more people get confused
    • agreed, if rustc does it, cargo should too
  • josh: can we disambiguate target and target (build target, platform target)
    • [target.().dependencies] for platform target + some additional cases
    • --target <triple> for platform target
    • --all-targets for build target
    • --target-dir for where to place build target output
    • See also https://doc.rust-lang.org/cargo/appendix/glossary.html#target
    • epage: iiuc "build target" is top-level item while we have "build unit" or "crate" for any element in the build graph
      • "crate" is another confusing word because people conflate it with "package", made worse by .crate files and crates.io
    • Last discussed: https://hackmd.io/Y9inxn_GQ0KzmGHwiXChcA#2022-11-22
    • can't really change target in terms of "platform", since it is used heavily in places like rustc's cfg values, and [target] tables in Cargo.
    • There isn't as much dependence of the term "target" for a cargo target, except for --all-targets, but we could potentially rename that CLI flag.
    • weihang: use "artifact" for a cargo target
      • josh: 90% perfect. "test" isn't quite an artifact, since it's less about the test binary and more about running the tests. "check" isn't an artifact at all.
      • ed: could help to keep test binaries in the output directory.
    • jacob: how does this interact with "artifact dependencies"?
      • Seems like it work well.
    • eric: artifact has some ambiguity with whether it is the thing that is created, versus the thing that created it.
      • josh: would call the thing that created it (main.rs or whatever) the "source of the artifact".
    • weihang: libraries can create multiple outputs, so does that mean the library artifact creates multiple artifact outputs?
    • eric: clarifying, if there is a "lib" artifact, what do we call the files created by the lib? Currently we call those individual artifact files. Conceptually I have a mental model that those are different things.
    • josh: What about "component"?
      • eric: That is what Cabal (Haskell) uses. But wasn't super enthusiastic about it.
      • josh: Already used in "rustup", too ("clippy" and "rustfmt" are "components")
      • ed: Similar to "node", that can be anything in the graph (roots or leaves).
    • ed: Should we bring it up on internals for feedback?
      • josh: can do it, but anticipate lots of paint colors.
      • ed: mostly for identifying any unknown problems.
      • josh: volunteers to post it.
      • josh: should post an issue first.
      • josh: should we FCP to record the decision?
        • yes
    • jacob: should we do a test-run of rewriting the documentation to find the problems?
    • josh: double-checking the artifact-dependencies
      • ed: artifact-dependencies just changes what artifact you depend on in a dependency.
      • weihang: also note: you can't depend on test artifacts.
        • ed: that's ok that there are some restrictions on which artifacts you can depend on.
  • ed: Should we add autolib?
    • https://github.com/rust-lang/cargo/issues/14476
    • For cargo-script, to turn off discovery, sets all auto* to false. Overlooked libraries. If you have a lib.rs, it tries to
    • josh: should there be an "auto=false" to turn them all off?
      • arlo: does anyone want that (other than cargo-script)? Is programmatic only OK?
        • ed: also publish. But may not help since this is used internally in the API.
    • ed: This should be part of Cargo.toml because that's how cargo-script communicates its behavior, and would be beneficial for consistency.
    • https://github.com/rust-lang/cargo/issues/5330
    • https://github.com/rust-lang/cargo/pull/5335
    • eric: thought the original just didn't have anyone asking for this
    • jacob: was thinking there was a resistance to deviation from the norm.
    • no objections noted

2024-09-17

josh: out

  • epage: Rendering of deps being behind
    • Behaviors
      • Original: no suffixes
      • When issue opened: say (latest: <VER>) (yellow) if any newer deps are found
      • Now (screenshot):
        1. (requires Rust <VER>) (red) if MSRV of dependent workspace members violated
        2. (available: <VER>) (yellow) if MSRV-compatible, SemVer-compatible upgrade available
        • Package wasn't selected (hidden without --verbose)
        • MSRV-resolver failed the user
        • Version requirement upper bound (note: SemVer-compatible, not dependent-compatible)
        • Missing feature
        1. (available: <VER>) (yellow if direct dep, plain if transitive) if MSRV-compatible, SemVer-incompatible upgrade available
        2. (available: <VER> requires Rust <VER>) (plain) if Semver-compatible upgrade available
        3. (available: <VER> requires Rust <VER>) (plain) if Semver-incompatible upgrade available
    • eric: would it be different if cargo-outdated was in-tree?
      • ed: Good to highlight things that have newer versions, but are constrained in some way.
    • jacob: if the resolver had better error messages, it could have a link for a command to reconstruct (a verbose version) of why it is unable to pick a newer version.
    • jacob: How hard to make it a table?
      • ed: Willing to do that. cargo-upgrade uses a table. Some concerns from people about using a table. But with distinct columns, it can be easier to scan.
    • arlo: Tools like dependabot might want to be able to machine-read this kind of information.
      • Though they could use some existing infrastructure to do that.
      • ed: Possible to do with cargo metadata, etc.
    • arlo: Confused about the colors, not immediately obvious what they mean, hard to see a pattern.
      • ed: Should it drop the uncolored lines? Subtle word changes?
      • arlo: Would like text differences for when they are colored differently.
      • eric: If a table, could have a note at the bottom of the table explaining the columns.
      • arlo: Could use letters as footnote labels.
    • arlo: Lots of ways things are not updated, and people are going to want to know why.
      • ed: Unfortunately can't easily answer those, yet.
      • arlo: Would prefer to not put reasons in if we're not confident it is correct.
    • ed: Could remove the uncolored lines. They are mainly there to highlight tech-debt.
    • ed: josh mentioned it would be helpful to include the rust version in all of the lines. Concerned over confusion of the plain case.
    • ed: Please reach out if you have ideas on how to tweak this.
  • ed: Does anyone fully understand the dynamic job pruning discussion?
    • jacob: Compare Rust to C, where in C you can modify something in the middle of the tree, and relink and everything works. Is it possible to teach rustc and cargo that certain kinds of modifications are ok to skip work of recompiling dependents.
    • arlo: Do we know if the compiler team would be willing to help provide the kind of hash needed for this.
    • ed: Probably best to wait for Piotr to post an explanation.

2024-09-10

RustConf

2024-09-03

  • epage: Adding build.rs features for build probes
    • See https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/.E2.9C.94.20-Zbuild-std.20with.20build.20scripts.20that.20invoke.20rustc/near/465984005
    • Point to cfg(version) cfg(accessible) instead?
    • eric: There are other use cases than build probes.
      • josh: artifact dependencies, though that might be going through cargo
    • eric: probably wouldn't want to put much effort into supporting build probes, but uncertain about the cfg support.
      • josh: Concern over cycles in the implementation. cfg(accessible) is probably mostly done, and just needs someone to stabilize it. cfg(version) is mostly there, but Josh doesn't want to see cfg(version) stabilized without cfg(accessible) to avoid excessive version checks rather than feature checks, but would like to have both.
      • cfg(version) doesn't really help for nightly. Nightly there is nothing we can do about.
      • For crates with really old msrv's, these features probably wouldn't cause them to drop build scripts.
    • ed: would be interesting to talk with david about whether or not msrv aware resolver would allow him to bump msrv and access these new features.
    • ed: pushing back on nightly probes, causes breakage in various ways.
    • josh: Feel like it's fine to close any issues related to build probes where there's no use case other than for version detection.
    • arlo: global features might be useful, so that something like "use nightly features" could be enabled across all crates that want that.
  • josh: build --all-targets should build doctests (and likewise for other commands)
    • https://github.com/rust-lang/cargo/issues/6669
    • Was looking at udeps. There's no single command to build everything (--all-targets does not do doc tests). This is unfortunate.
    • Would it break people?
    • New flag? especially if we rename "target"
    • mode vs target
    • How hard would this be to implement?
      • eric: uncertain, but this might actually be a little challenging. Would need to check if there are any global checks of the compile mode.
    • ed: had discussions about doctests be libtest tests.
    • josh: feels like mode doesn't work for doctests specifically.
    • josh: if adding to --all-targets might break everyone, we could add a new flag that would (if we rename "target").
      • ed: there are people who don't know they aren't doing doctests with --all-targets. Adding it could cause breakage they aren't expecting.
      • josh: Add new option, but make them aliases in an edition?
    • ed: could do an unstable flag to change --all-targets, and do a crater run to see the impact.
      • josh: even light breakage could be frustrating.
    • eric: what is the importance of a single command that does everything?
    • ed: can't check or clippy doctests.
      • Could be a pre-requisite if we want to change --all-targets.
      • --doc everywhere might be a blocker for --all-targets meaning change
      • eric: vaguely remembers work on this, but doesn't think it was implemented.
    • josh: short-term, can we add a --really-all-targets and then figure out a new name later?
      • ed: would this only be in cargo test?
      • josh: and cargo build and cargo check and cargo clippy and cargo fix ..
    • agreed to start an experiment with a flag that we would bikeshed at a later date.
    • josh: cargo fix is another one that could potentially work with doctests.
      • eric: That would have the same blocker as cargo check.
  • josh: Detecting unused crates across the whole crate graph - handling dependencies that are unused for some targets or sub-targets but not others.
    • rustc has a lint for detecting unused dependencies. But it does not work well in cargo, particularly with dev-dependencies.
    • https://doc.rust-lang.org/nightly/rustc/lints/listing/allowed-by-default.html#unused-crate-dependencies
    • rustc does not have enough information to solve that.
    • can rustc include information in json output so that cargo can aggregate the union for producing a final lint?
    • ed: would platform conditionals work?
      • josh: this would work for checking a single target.
    • arlo: are there other lints that could benefit from this kind of aggregation?
      • josh: there may be, but not sure exactly what that would be.
    • eric: uncertain how the user would get this warning
      • josh: cargo would decide. rustc would tell it what wasn't used. cargo would decide which ones depending on what was being built. Wouldn't warn about dev-deps.
      • eric: this seems like it would only work with --all-targets.
      • eric: struggingle to remember how cargo-udeps works. I believe it currently uses depinfo. depinfo already lists only the things that were used.
    • scott: wondering about how this would be presented as a lint. Where would it run? This wouldn't be checked all the time (such as only with --all-targets). Only makes sense in certain places. Would that be in cargo clippy?
      • ed: clippy and check still have the concept of targets, so it wouldn't be able to be dedicated in something like clippy.
    • josh: makes sense for build dependences all the time.
      • For regular dependencies, a normal cargo build or cargo check could do this, since it is doing lib and bins. Just suppress it if you are doing a subset (--lib or --bin).
      • The only interesting thing is dev-deps. If you pass --all-targets, you enable it automatically. Only enable when you are building enough to avoid false-positives.
        • eric: sounds good to me!
    • josh: Are there any objections to ask est31 if cargo would be able to do this with just depinfo?
      • eric: sounds great if they are interested (josh: yes they are)
    • arlo: wondering about cargo machete?
      • josh: probably want something that is precise.
    • https://github.com/rust-lang/rust/issues/57274 historically where we tracked this.
    • Josh to follow up with est31, and open an issue.

2024-08-27

  • ehuss: time inference breakage
    • In 1.80. alloc added new trait impl that broke "only one impl" inference, breaking time
    • Proposal to transparently patch older versions of time: https://github.com/rust-lang/cargo/pull/14452
    • Context:
    • Comments on patching the time source:
      • ed: feels like this is something that needs to be planned not under pressure, since there are unknowns and concerns about mistakes.
      • josh: clever and terrifying idea. Superficially doesn't seem horrific, but implications is a line that we haven't crossed before. rustc has had hacks in the past for particular versions of crates, but with the intent to be removed over time. It is not obvious how long cargo would keep a kind of hack (or if there is a time limit). It seems surprising when cargo doesn't build the original source. For example, if you want to test the breakage or a mitigation in some way, this would prevent doing that.
      • ed: t-types doesn't want to add hacks to support time specifically.
      • jacob: Has anyone asked why this doesn't deserve a revert and stable backport?
      • jacob: concern about vulnerability of adding more patches in the future. Would want a process and tooling to carefully audit those kinds of changes.
      • josh: very hesitant to do this in a rush.
      • josh: how close is unidiff to shipping?
        • weihang's PR was closed, ran into a lot of problems particularly with the resolver.
        • josh: In theory, it could just say "cargo.toml patches are not supported". However, that is one of the primary use cases of the feature (though it would work for this scenario!).
      • josh: Needs warning at bare minimum.
      • ed: Could break checksums as an example.
      • rustin: Concern if it would work with mirrors?
        • Uncertain if is_crates_io is true in that case.
      • arlo: Why is this coming up now?
        • There has been a steady stream of people updating to 1.80, and hitting the problem. Not a problem with crates in general. Long tail of people who can't just do cargo update. For example, a linux distribution (Nix) which can't just update (they are looking at improving that).
    • Comments on other approaches for time?
      • josh: It would be amazing if the compiler could detect a source file by hash, and fix the resolution. Unfortunately the compiler has less information than cargo does. Hesitant to want the compiler to hack around that.
        • If cargo had unidiff support, would love to lean on that. But in the absence of that, not going to advocate for that.
      • eric: was wondering if another option would be to detect the version of time, and display a message. But if rustc is doing that, may not really help that much.
        • ed: resolver-time message? This could be say something earlier in the process.
      • scott: Cargo could be intercepting the error and providing more context. There could be an interface between cargo and rustc for indicating that context needs to be added.
      • scott: If there is something reasonable that is not a permanent hack, we should probably do something. But hasn't heard any proposals that don't seem like a hack.
      • ed: sympathetic, doesn't feel like there is something we can do now, but would like to be proactive for the next situation.
      • josh: agree, with regrets, give some feedback about appreciating the effort to mitigate this, but we are concerned about doing something like this in a rush, and don't want to deal with the fallout of doing it in a rush.
    • Comments on other approaches for a general-purpose long-term solution
      • ed: If we had the unidiff patch support, then there could be built-in support, and perhaps a registry could also indicate those patches.
      • jacob: yes, once we have it for companies and in the RFC for crates.io could then address this, with the patches in a dedicated location with dedicated tools and processes
      • josh: Is this an argument that unidiff support could be prioritized?
      • josh: Other teams are looking at long term solutions as well (lang for inference, libs for the process, etc.).
      • rustin: Unclear if crates.io had a mechanism to patch things, wouldn't they need to publish something new?
        • jacob: No, this would be a global patch system, which would force older versions to apply the patch.
        • josh: Would likely need to involve both the maintainer and the registry maintainers.
      • jacob: t-release is working on improving the release notes process so that process will help in the future.
        • eric: new system is up. When a PR is labeled "relnotes", an issue is opened with a template where anyone can enter the proposed text for the release notes and the blog post, and then the relnotes tool will collect that.
      • ed: Acknowledge the steps for making incremental progress, for example rustin's yanked messages which could be a step towards the mutable registry database, which could support patches, weihang's experiments with patch support, get those things moving forward, and progress from there. Good to create an issue for a bigger solution.
    • rustin: how to deal with the PR?
      • eric: Think we will close it for now.
  • epage: MSRV-aware resolver in mixed-MSRV cases Part 2
    • See https://github.com/rust-lang/cargo/issues/14414#issuecomment-2311090563 for a short summary of the problem
    • See https://github.com/rust-lang/cargo/issues/14414#issuecomment-2311094250 for analysis of the solution
    • ed: See this as taking off one rough edge, but not necessarily all of them. Doesn't feel like this is making things worse (in terms of education).
      • Concern around the messaging about not going forward with this.
    • josh: What is the current state?
      • Currently works with workspace resolve with the lowest msrv.
    • josh: A workaround, would it be more appealing to default to the highest? The resolve would more likely to work, but they won't necessarily be easy to test the oldest msrvs without something like cargo-hack.
      • ed: How do we define the work the user needs to do? There are workflows that are more difficult with the highest. There are tradeoffs here, and on balance that approach looked to have worse outcomes overall.
    • arlo: Picking the solution that is most likely to work for users, and easy to explain to users seems best, and this seems to do that.
    • ed: There will be a message in cargo update (like cargo update --dry-run --verbose) that will show changed lines, red with MSRV incompatible, yellow is MSRV compatible but for some reason not picked.
    • jacob: This will just be another oddity of workspaces that can prevent updating individual packages.
    • rustin: With mixed msrv, what is the toolchain version to use in CI?
      • ed: All PR CI's on stable, and then a canary scheduled job for nightly, and a CI job that will cargo-hack and run each MSRV for each package.
        • arlo: Currently it is up to you to keep the lock file compatible, and this feature will make it easier to generate that lockfile so that it is compatible with everything.
    • ed: Are there any blockers for the bucket solution?
      • josh: Not blocking it right now, and want to discuss it further.

2024-08-20

  • rustconf
    • team dinner
      • When is a conference dinner to not conflict?
  • ehuss: target-applies-to-host regression https://github.com/rust-lang/cargo/issues/14253
    • weihang plans to look at it, will look at a revert if can't figure out what the fix is
  • epage: compile-time-deps (for IDEs)
    • Discussed on 2024-06-18
    • Unclear what the disposition was from the notes
    • Sympathetic to use case
    • People should have a way
    • First class or not is unknown
    • Problem is UI
    • More general flag to take in combinations of packages to build
    • One use case is an editor, or if you are working on a build script and want to build just the build script in a terminal.
    • This is a parallel to the other build targets have
    • josh: future topic: separate two meanings of target, phase one out
      • build target
      • platform target
    • proc-macros is a package selection flag
    • build-script is a build-target selection flag
      • --build-script
      • but that would be for building it, not also running
    • Package selection flag for groups
    • The two flags might not compose the same way as --compile-time-deps, requiring separate invocations
    • Query system like buck?
    • weihang: what commands is this available for?
      • run?
      • RFC currently just says cargo build
      • josh: If people want to build and run the build script, that's still part of "build"
    • Disjoint --compile-time-deps running build.rs but not proc macros seems specialized
    • cargo build --build-script
      • Almost like a cargo prepare
    • josh:
      • bikeshed=host
        bikeshed=build-script
        bikeshed=proc-macros
    • cargo build with options on where to stop
    • weihang: Could there be a perma-unstable option for them to try?
      • ed: Can r-a use unstable options?
        • weihang: yes, they are already doing it for the standard library.
        • ed: might be issues with r-a probing to detect if cargo supports the feature.
    • josh: contrast to build graph
      • that exposed all details and boxed us in
      • because of specialized nature, this doesn't box us in
    • ed: a concern about the proliferation of flags overloading the user/documentation with excess information
      • arlo: could be a hidden option
      • jacob: does a config flag help with this?
        • ed: Feels weird to affect behavior in this way.
        • josh: This isn't a "permanent" setting, so might not make sense as a config option.
    • ed: Not clear about the problem statement, is it primarily for performance?
      • arlo: We believe that is it.
    • scott: Feels like this is a specialization of a more general solution.
    • ed: If this is pressing enough, it could be a hidden flag.
      • Giving a short-term solution can prevent moving towards the big solution.
    • jacob: Ask them if an unstable option would unblock them, until we can make a decision if there is a more general solution?
    • ehuss: is this just performance or is there any correctness angle?
    • ed: Will follow up with asking about the indefinite unstable option.
  • epage: MSRV-aware resolver in mixed-MSRV cases
    • Effectively, this makes it so only your lowest MSRV has an MSRV-aware resolver, everything else resolves to maximal
    • We could track all msrvs, see https://github.com/rust-lang/cargo/issues/14414#issuecomment-2294068030
    • jacob: Given constraints, that's probably the best we can do.
    • ed: Planning to make the warning about "you need a newer MSRV" to specialize it for what is needed.
    • josh: Sounds like it might come close to the fully general solution, but may result in a more complex resolver.
      • Correct, it isn't path-dependent.
      • Wondering about the difficulty for the user to understand.
      • If we are not going to do the fully general solution, wondering if it can be simpler. If there is a workspace with multiple MSRVs, tell the user to stick something in the workspace for the MSRV to use.
      • jacob: The mix msrv's are fundamentally a conflict for what it is asking the resolver to do.
      • josh: It seems like it would be good to have a workflow where in CI, each individual package in the workspace using just that package's MSRV.
        • ed: We don't really have that, and that seems like a whole lot of features to design.
      • ed: Aware there will likely be some confusion by some users, but if this will likely address 80%+ of the cases.
      • jacob: Seems like a tension of whether or not to hide complexity (hiding complexity that the user would need to know about anyways).
      • josh: Would like to discuss this more.

2024-08-13

  • edition 2024 blocker: https://github.com/rust-lang/cargo/issues/14283#issuecomment-2273749304
    • https://github.com/rust-lang/cargo/pull/14366
    • epage: all changes have risk, this needs analysis for how much risk
    • josh: only touches nightly
    • josh: want another pair of eyes
    • scott: looked seems isolated
    • agreed to move forward
  • epage: Optimizing git patches / dependencies: read_packages
    • see also https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Redundant.20code.20in.20.60GitSouce.60.3F
    • epage: parsing all manifests in repo is slow
    • epage: we only directly discover manifests in root repo, not submodules, and not in any hidden dirs. Checking path dependencies gets us into those locations
    • epage: Could we cache the index? Could end up being cargo-version specific. Would rather this not grow. Likely fine to only cache the index
    • epage: reuse hash logic from rustc_info
    • someone: git commit
    • josh: dirty or not built from git
    • epage: could not cache when dirty?
    • someone: test suite cant test it?
    • someone: no, its not dirty
    • weihang: store in file would cause checkouts to be thrashed
    • epage: no, we'd still just check the file for checkouts
    • epage: trying to make sure cache gets cleaned up
    • josh: do we have data on whats slow? should we store manifest files?
    • epage: most time is the rest of the parse process
    • epage: if we optimize parsing, store in different format for immutable sources
    • josh: should we just add support for the path in the dep
    • epage: thats opt-in, couples people to the layout, and more annoying to add the dep / patch
    • josh: should we cache in Cargo.lock
    • jacob: which cases do we care about?
    • epage: Cargo.lock exists
    • jacob: would help with ambigious name warning
    • epage: we'd need to figure out if this works through all the right layer
    • jacob: most wouldn't be redundant if we add the dep field
    • epage: the potential "get it from Cargo.lock" complexity wouldn't go away
    • josh: could we just re-org cargo repo
    • epage: this was zed which doesn't depend on cargo that I saw this
    • https://github.com/rust-lang/cargo/issues/14395
  • rustin: For the yank message feature, should we continue to use the DELETE and PUT HTTP methods, or should we change it to the PATCH method? https://github.com/rust-lang/crates.io/issues/9193#issuecomment-2271195461
    • crates.io views the current API as "weird"
    • Currently, yank -> delete, unyank -> PUT
    • Registry changes, breaking change
    • epage: can we do this without a breaking change?
    • rustin: weird to have content in a delete
    • rustin: crates.io doesn't want to add to the old API
    • jacob: "we don't care, defer to crates.io"
    • jacob: weird to have different endpoint and method, lots of flexibility
    • rustin: described the API
    • jacob: that seems like a large API surface
    • jacob: want to be able to change smaller parts, no disambiguating
    • josh: getting tied to REST semantics / verbs
    • josh: PATCH <name>/<version> is weird, be more specific like PATCH <name>/<version>/yank
    • epage: what we do care about is supporting multiple registries
    • weihang: condition which API is used by whether message is used
    • jacob: have the registry tell us whether we can use it
    • https://doc.rust-lang.org/cargo/reference/registry-index.html#index-configuration
    • rustin: we don't need a new endpoint, just say you can't
    • TODO: write up the API proposal
  • epage: https://github.com/rust-lang/cargo/issues/13873 mixed MSRV workspaces

2024-08-06

jacob, ehuss out

  • 1.80.1 and 1.80.2
  • josh: cargo update --breaking
    Image Not Showing Possible Reasons
    • The image file may be corrupted
    • The server hosting the image is unavailable
    • The image path is incorrect
    • The image format is not supported
    Learn More →

    Somewhere between deferred and rejected (speaking for myself): Support in cargo update for writing to the manifest for non-breaking changes, like bulk compatible upgrades of version requirements (ie a -save flag) which was one of our lower priority workflows. A save flag is more about updating versions for your dependents, which while important for having valid lower-bounds on version requirements, doesn't fit with the existing model of cargo update. Maybe in the future we can find a way to express this in cargo update that fits with how it works or maybe another command can take on this role. We just aren't wanting to distract our efforts for handling most of the use cases to handle this one'

    • options
      • --save
        • ambiguous name
        • Need a name that fits with a clear mental model
        • If also config, need a way to turn it off
        • TODO check the design guidelines
        • TODO double check previous concerns
      • lint on locked newer than reqs
        • cargo fix
        • auto-fix on cargo update
        • See also "a config"
      • A config
        • some projects allow unrelated changes (accidental changes) to the lockfile, but not the manifest
        • lockfile only causes easier to resolve conflicts (nuke and regenerate), manifests are more of a pain (actual merging required)
        • people who maintain two lockfiles
      • cargo update -Zminimal-versions
    • dirty tracking?
      • cargo add, cargo remove, cargo update --breaking (unstable) don't
      • cargo fix would do this anyways
        • cargo update auto-fixing is a gray area
  • epage: minimal versions
    • epage: at an impass, we worry the ecosystem isn't ready but not having it is making ecosystem adoption harder
      • josh: whats the problem?
      • epage: ensure transitive dependencies are updated
      • josh: who will feel this if everyone is always on latest deps?
      • josh: --save should be the encouraged default, so version reqs are latest; if someone wants an older version of those dependencies, they can use an older version of your crate
      • josh: "drag manifest forward rather than dragging lockfile backwards"
        • epage: possible because of MSRV aware resolver and update --save config
        • epage: end users are the people hurt and less likely to use non-default settings
        • josh: the more popular a dependency is, the more likely at least one of your transitive deps will fix it
        • use cases: reduce update churn, allow people to hold back deps to workaround bugs
        • https://github.com/rust-lang/cargo/issues/14372
    • epage: use cases
      • version req testing ("will I force a version bump in my dependents when I use something new")
      • want version reqs to match lockfile
    • epage: we should rip the bandaid off, but which approach
      • -Zminimal-versions is easy to describe at a high level
      • however, that description will either not do what people want or, worse, people will think it does what they want (minimal version testing)
      • anything (transitive or direct dep) can have a version higher than what you want, messing up minimal version testing
      • -Zdirect-minimal-versions is harder to describe at a high level
      • however, it fits perfectly for minimal version testing
      • however, its a pain because you need your version reqs to be the highest of your dep tree (it effectively pins to the lowest version)
      • maybe we could describe this with multiple fields in config?
      • maybe some judicious cargo update logic could make -Zdirect-minimal-versions less of a pain
    • TODO post on the tracking issue a proposed forward to collect
  • weihang: why cargo rustc --crate-type doesn't allow binary?
  • epage: Optimizing git patches / dependencies: update_submodules
    • epage: No-op update_submodules is slow (walking whole repo?)
    • epage: Maybe gix could make a meaningful perf difference
    • epage: We do short-circuit clone_into with the existence of a .cargo-ok, maybe we could write that after update_submodules, allowing us to skip the call
      • Old cargo should work fine with this
      • If an old cargo got interrupted mid update_submodules then new cargo won't correctly re-checkout the repo.
      • Eventually, this will go away. Rare enough to "ok"?
    • arlo: have transition period where the file has content
    • epage: is there enough of a case to be worth it?

2024-07-30

  • epage: https://github.com/rust-lang/rfcs/pull/3371 and https://github.com/rust-lang/cargo/issues/14125
    • Approach 1 (RFC): target-dir is for intermediate artifacts, artifact dir is for final artifacts
      1. Make target-dir movable (RFC 3371)
      2. Stabilize artifact dir (#6790)
      3. Allow templating in artifact dir and have it responsible writing the runnable binary, etc
      4. Change the target-dir default to point to central base path, like cargo script, leaving artifact-dir writing to target/
    • Approach 2 (kornel): target-dir is for finals, private/unspecified dir is for intermediates
      1. define a new intermediate artifact dir in a central base path, like cargo script
      2. Slowly migrate intermediates out of target/ to this new directory
      3. Reject --artifact-dir (#6790), saying --target-dir solves that need
    • epage: Approach 2 is elegant but we lose a big benefit of --artifact-dir: no nested, harder to predict directories before getting to artifacts
    • epage: both suffer from "what is intermediate? what is final?"
    • josh: assumption: want to name the intermediate directory and control it. The main difference between the proposals is what to call them? Seems like a problem of naming.
      • With approach 2, doesn't need a command-line flag. Could also resolve the problem of how the layout changes with cross-compilation.
      • josh: What happens with artifact-dir if there are multiple targets?
      • ed: Will had multi-target to the artifact-dir issue to be resolved.
      • josh: Would be nice to be consistent with the layout between "host" and "cross" and "multi-target".
      • ed: One of the use cases for artifact-dir is to place things in their final location for packaging.
      • weihang: Does it need to consider profile?
      • weihang: Could there be a flag to output the expected output directory?
        • Like buck2 analyze, to print out the specific paths for a specific kind of build.
    • CARGO_TARGET_TMPDIR could potentially be not in the target directory. Just a confusion around the name (since it has "CARGO_TARGET" in the name).
    • josh: Template component could include the platform-target, so that it is possible to keep things separate. For multitarget builds, we could detect if you haven't included that template component, and error out.
    • josh: approach 2 could have a smoother transition.
    • jacob: Is there still going to be a need to copy final artifacts to an arbitrary location?
      • They are mutually exclusive options.
    • josh: There is convenience of "just put the final binaries here". If there is a transition, could take the opportunity to structure things differently.
    • ehuss: problems
      • difficult to have profile agnostic code
      • difficult to be agnostic of host build vs cross-compiling
    • josh: do both
      • Move intermediates
      • Still provide a templatized "artifact" dir (whatever we call it), to make it easy to place binaries in one place, or in the multitarget case, map them to specific locations.
    • Ed: Could keep it in config-only to convey it is an advanced option to avoid cluttering the CLI argument.
    • Josh:
      • Target dir will become obscure, hide it in -h, maybe man as well; just have artifact dir option
    • ed: Should we hold off on templating if there is an artifact option?
      • josh: I think we do need a coherent plan.
      • jacob: Does it make sense to stabilize templatization, and then when there is a new variable it can be templatized as well?
      • jacob: Difficult to say, blocking for years for some final solution, or some shorter-term incremental solution.
        • ed: a concern de-incentivizing the final solution.
    • josh: Internally cargo will need a way to define where intermediate artifacts go.
    • jacob: eventually want target-dir to only be final artifacts.
      • target-dir continues to be "everything".
      • If you specify artifact-dir, then target keeps whatever is left.
    • Could have a transition plan, where there the eventual goal is to have an "artifact dir" and an "work dir", and the "target dir" is historical.
      • Could have templating for artifact-dir and work-dir, and not target-dir.
    • jacob: Could time-box the solution, where if it takes too long.
    • hide target dir, have two templated options artifact-dir and work-dir. Postpone the current target template rfc for 1 year (if no other progress). unstable work-dir templated option, which would default to target-dir.
    • weihang: work-dir is a confusing term.
    • work-dir would help transition people off from target-dir.
    • artifact-dir helps with being profile and target agnostic.
    • a problem with target-dir template, there isn't a good way to tell it to "don't create sub-directories".
      • Could say, if there are no template variables, that it creates to present-day subdirectories.

2024-07-23

  • jacob: https://github.com/dtolnay/semver/pull/321
    • If you have a chance to look, trying to determine what the semantics actually should be.
  • epage: more implicit feature fun
    • https://github.com/rust-lang/cargo/issues/14283
    • Concerned over edition complexity growing in the TOML normalization code, especially if we then "simplify" things next edition with more feature changes but leaves us stuck with 2024 edition code.
    • josh: Would it make sense to make foo/bar fail if foo is not optional?
      • ed: That is the required way to set features on a dependency.
      • josh: We could require that you use foo?/bar if foo isn't optional.
    • Could just allow the dep: for non-optional?
    • epage: move feature normalization to summary generation
    • arlo: Would it need to wait for the next edition if it was cut?
      • ed: yes
    • eric: Would prefer to try to keep it, can help to look at the code to see what kind of options there are.
    • josh: If a?/b worked with non-optional, that makes the syntax a little strange since the ? was to mean "maybe enabled". dep:foo/bar could be foo?/bar, at which point we could migrate both dep:foo and dep:foo/bar to a new syntax without dep:, like deps = [...]
    • weihang: dep:namespace::pkg/feature.
  • eric: checking in on msrv config decision
    • ed: not had time to workshop it
    • will likely work on a PR soon.
  • arlosi: next steps for https://github.com/rust-lang/cargo/pull/12875
    • we had a contributor offer to help move it forward
    • possibly blocked on lint configuration
      • epage: I see these as unrelated
    • epage: see https://github.com/rust-lang/cargo/issues/8424#issuecomment-1915265827
    • epage: see https://github.com/rust-lang/cargo/issues/8424#issuecomment-1915272203
    • arlo: Do we generally want the ability?
      • Turning on in CI, want to see that it was a warning turned into an error, don't discard useful information.
    • No recompile happens with this.
    • arlo: The implementation could be better, but the current one should work.
    • josh: we want people to adopt this. If it is documented well, people can copy that. Putting in the CLI makes it easily discoverable.
    • josh: skipping the bikeshed by doing config first, and then CLI later can help it deliver faster.
    • weihang: What is the scope of the flag? Cargo or rustc warnings?
      • arlo: Currently it is everything.
      • ed: Felt that it could be extreme for all of cargo's warnings to be denied like that. Propose to be more intentional, just rustc.
        • rustc only
        • rustc + cargo linting
        • rustc + cargo shell-warn (all warnings)
      • arlo: Does the user need to be able to configure between those?
      • ed: Since it is ok to add new warnings in a release, then it should be ok to change the meaning in future releases.
      • arlo: Concern over just rustc, couldn't document that "all warnings".
        • ed: Could just "turn warnings into errors", and most people probably won't think about it.
      • jacob: Could declare it a bugfix :D
      • arlo: Likes rustc + cargo-lints
      • josh: would like it to eventually cover everything, but ok to be incremental.
      • jacob: Do we want to document it with a footnote as to what it affects?
      • ed: Could start generically, and say "warning lints", since there are no stable warning lints in cargo right now.
      • josh: Feels like most cargo warnings should raise to an error, and thinks things like --dry-run will be a rare exception.
    • config option naming?
      • build.warnings?
      • some resistance to term.warnings, felt weird since it isn't a terminal setting.
    • What options for the config?
      • "allow"/"deny"
      • "allow" would hide warnings
      • Would need a default. "warn"
      • Josh: What is the use case for "allow"?
        • During development, some people want to hide all warnings, and then turn it on later.
        • josh: Could that be in [lints]?
          • That is intended to be more of a static setting, not something that swaps around.
        • josh: Uncertain about encouraging people to set it to "allow".
      • scott: Could it just be unset means normal?
        • People want the ability to override it back the default.
      • scott: warnings="warn" feels a little weird.
    • weihang: Will this be unstable?
      • ed: Should probably be unstable for now, and ignore it if -Z... isn't specified.
      • eric: Would be good as a conservative option, and try to stabilize quickly. Good to get a little bit of public feedback.
    • josh: When stable, we should send PRs to places that have recipes for calling cargo from CI, and add this there.

2024-07-16

  • weihang: https://weihanglo.tw/posts/2024/the-missing-parts-in-cargo/
  • eric: Stabilize global automatic cache cleanup.
    • Proposal: Stabilize the automatic cleanup with just the gc.auto.frequency config option. Change the thresholds to 6 months, and then in 3 months lower them back to 3 months for downloaded content and 1 month for generated content (extracted files).
    • josh: Is this only for items for old items that haven't been touched?
      • Correct.
    • josh: Why would change it and then back?
      • Current values are arbitrary
      • How often use the project and care about latency
      • If date is too long, cache may grow too large
    • jacob: good for stabilizing. Doesn't seem worth the effort to change the thresholds. Or doesn't have to be planned. Or an awkward 3-month period would also be ok.
    • ed: 6-months seems fine, and re-evaluate later. The amount of usage in my system is 11G, and doesn't bother me.
    • weihang: For -Ztrim-paths, may need a new stable hasher, that may need a new PR.
      • ed: The path shouldn't be a problem, since it is always tracked (could double the space).
    • arlo: Suggest not changing the date.
    • scott: Easier to relax things, than making it more aggressive. 3-months make more sense.
    • ed: Does setting the config value break older cargos, is it ignore.
      • eric: Pretty sure it is ignored, but will check on it.
    • arlo: Does it do an unused key?
      • ed: Config doesn't do that, just manifests.
    • josh: In future, may want more creative heuristics, like keeping the latest version of a major series, and more aggressive threshold of older versions.
    • scott: Does it run if you are offline?
      • eric: no, if offline doesn't do that.
    • Conclusion: will stabilize next week-ish.
    • josh: is there some way to download things without creating a project?
      • eric: Yes, cargo-prefetch will download things. https://crates.io/crates/cargo-prefetch
      • josh: Should that be upstreamed?
      • eric: There are some hacky parts, like the top crates is hard-coded.
      • eric: Uncertain about how much demand there is for offline support is, don't see much, but not a scientific view
      • weihang: people don't use offline often, but when they do
      • ed: would there be a specialized command, or something more general (like cargo fetch option, or a general registry command that makes it easier to work with registries).
      • ed: could put cargo-prefetch in the offline docs. But not supported natively.
        • josh: out-of-tree would be an intermediate step.
  • epage: TOML bug
  • eh2406: Make Summary Sync https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Make.20summary.20sync
    • For testing the resolver code in many threads (for pubgrub). Rc prevents that.
    • Wants to load all the copy in advance, just one copy, and then run resolutions in parallel against that shared data.
    • ed: Probably Send is what you really need?
    • josh: doesn't feel like we should change to Arc in general for all users, but a debugging feature that did something like use Arc as Rc.
    • ed: If this isn't performance-noticeable on a large project, then go for it without an option.
    • arlo: Is this a breaking change for the public api?
      • We break in every release.
    • jacob: Didn't make a noticeable difference on a powerful linux machine. May be more architecture dependent, not os.
    • eric: are there any architectures that are particularly bad?
      • don't really know
      • scott: risc-v is one that could be an issue.
      • ed: How practical it is.
    • josh: expect it to be negligibly for all mainstream architectures.
    • josh: is this a long-term change, or is it something that can be backed out afterwards?
      • jacob: probably still useful over time.
    • eric: is there a branch?
    • josh: if it is trivial to make a feature flag, I would do that. but if that is non-trivial or ugly, then probably fine to use Arc.
  • epage: lockfile-path
    • epage: Can be a workaround for RO Cargo.lock preventing cargo metadata
      • Could point it to a temp directory.
    • arlo: another usecase is cargo install with a different lockfile.
    • eric: Uncertain if people having issues with readonly would be happy with that solution.
    • josh: Would it be an option for the file location, or the directory, or an option to not write it at all?
      • arlo: For some the path might be /dev/null.
        • jacob: that could be special-cased in cargo to work on all platforms
    • ed: overall seemed like a niche usecase. Probably don't want to streamline the workflow for that.
    • arlo: could this be an err-by-default lint that they could turn off?
      • ed: Currently can only be statically set, if it is a read-only source, they can't change it.
    • jacob: would like the cli option to be consistent with manifest path, and could change it later.
      • jacob: if it is ever weakened, probably should only be on the command-line.
    • ed: will start an unstable option assuming it is relatively trivial.
    • josh: can cargo detect the read-only filesystem error code, and suggest the option?

2024-07-09

  • FCP Reminders:
  • RFC 3663 - structured syntax for feature dependencies on crates
    • Seeking feedback for the right design.
    • Should this also include x?/y support?
    • ed: a balance of the DSL and TOML syntax. Concern that this will be a source of churn, and not a significant improvement?
    • scott: Can this be delayed until removing the / syntax (over ?/)?
    • ed: Would the DSL be deprecated? Not quite ready to say to do that.
    • josh: Wasn't intending to deprecate the DSL. Wanted to incrementally avoid requiring the DSL. One reason for delaying the a/b syntax, was to figure out how to express that.
    • jacob: Was assuming cargo add or rust-analyzer or whatever would automatically switch to the structured form. Or switch it back to the DSL.
      • scott: would only have the transition one-way to the structured.
    • josh: Current state of the DSL is awkward in places. The consistency issue of dep: and dep/feature. The shiny future is deps in one list, and features in the other. Eliminate the need for the ? syntax, and just have a/b.
    • scott: If designing the features table from scratch, would you go with the DSL or the expanded format?
    • jacob: Are we going to need more syntax in the future? For example, mutually exclusive or other such additions.
      • ed: Uncertain how this schema would be able to support that. Could result in a variety of different fields, which could get complicated. Concerned about with how that would work (with either structured or not).
      • josh: Doesn't want to further extend the micro-format. If it evolves to require expressions A mutually exclusive enum could define the values, each with an enables and a deps.
      • ed: Another example is https://github.com/rust-lang/cargo/issues/6789.
    • josh: One of the goals of this RFC was to make it easier to expand the syntax.
    • jacob: Should this be postponed until we see the shape of the next addition that will demand it?
      • josh: not the worst outcome, but would like to be a little more proactive.
    • ed: Would like to use this structured syntax without the DSL (without the ?/ syntax). Would like to see some demos of real-world features tables to see what it looks like. How would I interact with this, would we want to completely get rid of the DSL?
      • josh: what is the plan (over an edition) to deprecate the / syntax, and give the ?/ syntax a single-character syntax?
      • ed: ok to deprecate / now, and remove in the next edition. Wouldn't want to swap ?/ to /, since that could be confusing. Would want a transition period of one edition for that.
      • jacob: uncertain about the complexity of documenting both forms, versus the confusion of changing.
    • weihang: Is it possible to explain the syntax in plain english?
      • ed: planning in implicit feature documentation to cover this.
      • weihang: some tool to help the user understand what it means.
    • josh: Feels like I have enough information to rewrite for the complete picture. Here's what it would be like without the microformat, examples of real-world manifests, hypothetical additions with and without the microformat, etc. Main reason didn't, didn't want to bikeshed on those.
      • scott: Would be helpful to define the terms used to make it easier to discuss.
      • josh:
        Image Not Showing Possible Reasons
        • The image file may be corrupted
        • The server hosting the image is unavailable
        • The image path is incorrect
        • The image format is not supported
        Learn More →
  • epage: quick question on base paths
    • Regarding the manifests: clarifying the reason it is in the manifest is because it is part of the project, and not the environment.
    • The manifest "should" be able to be self-contained (not "must").
      • josh: had no problem with defining in config.
    • ed: still feels there are questions to be answered about how it interacts with manifests.
    • ed: [patch] table entries are similar to a dependency, and we would probably need to either reject it, or do extra work to support it. Are we going to explicitly reject [patch] for now?
      • jacob: Seems straightforward to support, but also fine with requiring the path to be explicit.
      • josh: Ok either way.
      • Seems like deferring this is good to keep it focused for now.
      • weihang: Doesn't think supporting [patch] would be that hard. But fine if we don't block it.
      • eric: feels like supporting it in [patch] should "just work". We would have to have extra work to not support it.
      • ed: Just felt that testing it and such would have extra weight.
      • josh: Add a GitHub suggestion for an addition to it.
      • ed: will resolve the [patch]
    • ed: Felt sketchy about the "target" swapping, where it checks out different things in the same location.
    • ed: will work to resolve final part.
    • weihang: How will this handle publish?
      • eric: Path's can't be published.
      • We'll probably want some tests to make sure it gets stripped and such.

2024-07-02

out: epage

  • scott: Should annotate-snippets for rustc be a project goal?
    • eric: Sounds like a compiler-team goal?
    • jacob: cross-team support is one of the benefits of having the goals.
    • josh: getting something submitted this week should be fine. Just need to get teams on-board. Probably want to check with compiler-team before their weekly meeting.
      • Having you as an owner under the compiler-team seems like a good fit.
  • eric: checking to see if everyone is OK with the change to add fingerprint
    • weihang: will it support build-script?
      • eric: currently explicitly deferred out
      • weihang: would be good to have that supported
      • jacob: build scripts would need to have the same TOCTOU support that rustc has. If it just collected the hashes independently, then it still has the same race problem.
    • eric: only slight concern was the fingerprinting code is a little complicated, and this may make it a little more complicated. Overall very positive for the general idea. Also uncertain about how output mtime checks will work.
    • arlo: only concern was supporting multiple hashes from debuginfo, and doesn't think those should be used.
      • eric: the proposal includes the hash algorithm in the CLI flag, and the author is investigating blake3 (for performance). I think just supporting one (good) hash algorithm should be sufficient.
  • RFC 3529 - Add named path bases to cargo
    • FCP proposed, checkboxes at https://github.com/rust-lang/rfcs/pull/3529#issuecomment-2176336500
    • Further discussion was done about whether or not should be allowed in Cargo.toml or just config.toml? RFC author reverted the change to add it to Cargo.toml. Does anyone have strong feelings about that?
      • scott: What happens if the base path isn't found in the config?
        • arlo: it won't work. Similar to registries have to be specified.
        • arlo: kinda likes the option, but doesn't see it being needed.
        • jacob: this is a two-way door that we can change later.
      • josh: would like it in the manifest, but won't block on it.
      • josh: would prefer generally to have all config be available in manifest. It would be self-contained.
      • arlo: Would you also want registries in the manifest?
        • josh: yes
        • josh: some things like rustflags would be debateable, such as replacing those with first-class support for those options.
      • jacob: seems like a reasonable request, when someone has a use case where it would be useful, seems like it would be good to add it. But not yet convinced to add it without the specific use case.
      • josh: my use case, I don't need to override it. Dislikes having out-of-sight/out-of-mind problem, where 90% is in the manifest, and then some things in config.toml. It becomes invisible (in part since it it hidden and separate).
        • josh: With 8 different crates in a workspace that have inter-dependencies, and then other non-workspace crates that are from a parallel directory. Those end up ../workspace/...... Would be nicer to have base="workspace". Another is instead of having path="../common/....", would prefer to have that ../common part be more explicitly written. There would be a split of client and server, with common parts. Some are public (open-source) repository, and other parts are closed.
        • jacob: also helps with restructuring a workspace, so you don't need to rewrite all the relative paths.
    • scott: Does this support recursive detection from a root directory?
      • josh: no. Would like to have similar support like how git deps work. Would be really nice if in intra-workspace that you could just refer to packages within the workspace.
      • arlo: Just want to be careful if having recursive support to consider the performance concerns.
    • Discussed how to move forward with the extension of supporting Cargo.toml.
      • eric: would prefer a lighter-weight decision, and not hitch it to a larger project (such as migrating all config.toml options). Perhaps just opening an issue describing the use case, and we can decide on it independently?
      • We may also discuss more when ed is back, since it seems like there was mostly a question around not knowing what the use case was.