Skip to content

Conversation

zhassan-aws
Copy link
Contributor

The git log command used in the toolchain update script generated commits that are not between the current and next toolchain commits. This PR fixes the command through adding the --ancestry-path option:

       --ancestry-path
           When given a range of commits to display (e.g.  commit1..commit2 or commit2 ^commit1), only display commits that exist directly on the ancestry chain between the commit1 and commit2, i.e. commits that are both descendants of commit1, and ancestors of commit2.

Resolves #4128

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

@zhassan-aws zhassan-aws requested a review from a team as a code owner June 5, 2025 21:47
Copy link
Contributor

@carolynzech carolynzech left a comment

Choose a reason for hiding this comment

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

Nice debugging! Two questions:

  1. Have you tested this on your fork?
  2. I'm a bit confused how this works, since from my reading about this flag it seems to be useful to filtering out history introduced by merge commits, and Rust has a no merge policy. But perhaps bors introduced merge commits during rollups? Or I'm misunderstanding something here.

@carolynzech
Copy link
Contributor

carolynzech commented Jun 5, 2025

I suppose more generally I'm curious why we're only discovering this issue now; perhaps the upstream history just changed in such a way to introduce this? This isn't critical to answer before merging if the root cause is unclear and we're confident that this fix includes all the commits that we need.

@zhassan-aws
Copy link
Contributor Author

Have you tested this on your fork?

I've tested it locally to verify that it lists the correct commits:

$ echo $current_toolchain_hash 
5d707b07e42766c080c5012869c9988a18dcbb83
$ echo $next_toolchain_hash 
59aa1e873028948faaf8b97e5e02d4db340ad7b1
$ git log --ancestry-path --oneline $current_toolchain_hash..$next_toolchain_hash
59aa1e8730 Auto merge of #141229 - tgross35:builtins-josh-subtree, r=Kobzol
a124fb3cb7 Auto merge of #141961 - matthiaskrgr:rollup-r09j2sp, r=matthiaskrgr
aae43c4532 Auto merge of #136942 - Kobzol:stage0-sccache, r=jieyouxu
3772a16207 Rollup merge of #141956 - bjorn3:minor_cg_ssa_cleanup, r=oli-obk
757d0e70d6 Rollup merge of #141931 - ArtemIsmagilov:issue-141849_2, r=nnethercote
209be9a1d4 Rollup merge of #141923 - rustbot:docs-update, r=ehuss
e61cd1485a Rollup merge of #141918 - ArtemIsmagilov:issue-141849, r=nnethercote
72f60fb700 Rollup merge of #141914 - onur-ozkan:follow-ups, r=Kobzol
5d32b5cad1 Rollup merge of #141861 - jieyouxu:windows-server-2025-20250527, r=Kobzol
4fa33eb8d0 Rollup merge of #141833 - Kivooeo:test-reform1, r=jieyouxu
19437666d9 Rollup merge of #141724 - Sol-Ell:issue-141141-fix, r=nnethercote
2e8401ae5f Remove type_test from IntrinsicCallBuilderMethods
00a88b903d Remove get_dbg_loc from DebugInfoBuilderMethods
59d993b6c7 use better default stage for `check::Std` when stage isn't explicit
2f176126aa Auto merge of #141954 - matthiaskrgr:rollup-zptd6t9, r=matthiaskrgr
2fa33b0624 Rollup merge of #141949 - onur-ozkan:move-test-float-parse, r=Kobzol
71009058b0 Rollup merge of #141936 - WaffleLapkin:report-in-deps-decoupling, r=oli-obk
94da3d9c70 Rollup merge of #141930 - Urgau:triagebot_concern, r=Kobzol
7e8a8c9cb1 Rollup merge of #141921 - ehuss:arm-min-max, r=tgross35
8fbf365f47 Rollup merge of #141898 - LukeMathWalker:patch-1, r=aDotInTheVoid
1e1e1885a6 Rollup merge of #141881 - lnicola:sync-from-ra, r=lnicola
ad876cbe86 Rollup merge of #141843 - fee1-dead-contrib:ast_visitor_visit_id, r=oli-obk
2da4bae07d Rollup merge of #141817 - mati865:fix-system-libs-when-cross-compiling, r=cuviper
807778ab78 Rollup merge of #141554 - Noratrieb:document-codegen-opts-better, r=bjorn3
9505178d50 run `x check` on mingw-check-2
ecc7dde80a handle stage0 on `Std::check`
59fbe04a52 move `test-float-parse` tool into `src/tools` dir
8a65d9febb make library profile to use stage 1 on `x check`
4e71f24a6b make `x check` to use stage0 by default
c68032fd4c Auto merge of #141944 - matthiaskrgr:rollup-e7xhp6w, r=matthiaskrgr
f3622ead6e Rollup merge of #141891 - jdonszelmann:fix-141764, r=jieyouxu
d7fcb09da7 Rollup merge of #141889 - ferrocene:lw/missing-dyn-kw, r=petrochenkov
f61645325e Rollup merge of #141886 - ferrocene:lw/2015-edition-directives, r=compiler-errors
69ebe39cea Rollup merge of #141876 - compiler-errors:missing-let-ty, r=SparrowLii
6a5459e186 Rollup merge of #141873 - neeko-cat:patch-1, r=tgross35
8db6881620 Rollup merge of #141741 - nnethercote:overhaul-UsePath, r=petrochenkov
aed1171c66 Rollup merge of #141677 - azhogin:azhogin/async-drop-unexpected-type-instead-of-drop-fn-fix, r=oli-obk
55f7571a7e Rollup merge of #140715 - lukaslueg:oncecellsyncdocs, r=tgross35
b17dba4518 Auto merge of #141210 - RalfJung:miri-std-doctests, r=saethlin
99426c570e Auto merge of #141750 - Noratrieb:gold-rush, r=bjorn3

Copy link
Contributor

@carolynzech carolynzech left a comment

Choose a reason for hiding this comment

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

Thanks! With the local test, I'm satisfied. I'm still a bit confused about the git wizardry but let's get this unblocked 😄

@carolynzech carolynzech enabled auto-merge June 5, 2025 22:32
@carolynzech carolynzech added this pull request to the merge queue Jun 5, 2025
@zhassan-aws
Copy link
Contributor Author

I'm a bit confused how this works, since from my reading about this flag it seems to be useful to filtering out history introduced by merge commits, and Rust has a no merge policy. But perhaps bors introduced merge commits during rollups? Or I'm misunderstanding something here.

I suppose more generally I'm curious why we're only discovering this issue now; perhaps the upstream history just changed in such a way to introduce this? This isn't critical to answer before merging if the root cause is unclear and we're confident that this fix includes all the commits that we need.

TBH, I'm not 100% clear. My understanding is that git log A..B without --ancestry-path should include all the commits that are ancestors of B but not ancestors of A. This should be what we want unless history is not fully linear, which appears to be the case here.

For example, the $next_toolchain_hash has commit 773fd1a707 as an ancestor, while $current_toolchain_hash doesn't:

$ git log $next_toolchain_hash --oneline | grep 773fd1a707 
773fd1a707 initial commit
$ git log $current_toolchain_hash --oneline | grep 773fd1a707
$

and this commit is from 2016:

$ git show 773fd1a707
commit 773fd1a707f5abff63c4fd47b73580faba65d468
Author: Jorge Aparicio <japaric@linux.com>
Date:   Sun Aug 7 15:58:05 2016 -0500

    initial commit

Why are there such old commits that are ancestors of $next_toolchain_hash but not $current_toolchain_hash? No clue.

@zhassan-aws
Copy link
Contributor Author

Looks like there is something special about this particular commit:

 git show $next_toolchain_hash
commit 59aa1e873028948faaf8b97e5e02d4db340ad7b1
Merge: a124fb3cb7 aff21f659f
Author: bors <bors@rust-lang.org>
Date:   Tue Jun 3 19:52:05 2025 +0000

    Auto merge of #141229 - tgross35:builtins-josh-subtree, r=Kobzol
    
    Merge `compiler-builtins` as a Josh subtree
    
    Use the Josh [1] utility to add `compiler-builtins` as a subtree, which
    will allow us to stop using crates.io for updates. This is intended to
    help resolve some problems when unstable features change and require
    code changes in `compiler-builtins`, which sometimes gets trapped in a
    bootstrap cycle.
    
    This was done using `josh-filter` built from the r24.10.04 tag:
    
        git fetch https://github.com/rust-lang/compiler-builtins.git 233434412fe7eced8f1ddbfeddabef1d55e493bd
        josh-filter ":prefix=library/compiler-builtins" FETCH_HEAD
        git merge --allow-unrelated FILTERED_HEAD
    
    The HEAD in the `compiler-builtins` repository is 233434412f ("fix an if
    statement that can be collapsed").
    
    [1]: https://github.com/josh-project/josh

@carolynzech
Copy link
Contributor

Looks like there is something special about this particular commit:

Ah, this makes sense to me, this seems to do some merging. Thanks for the investigation!

Merged via the queue into model-checking:main with commit bd51c78 Jun 5, 2025
26 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Jun 9, 2025
These are the automatically-generated release notes:
```
## What's Changed
* Toolchain upgrade to nightly-2025-05-04 by @thanhnguyen-aws in #4059
* Automatic toolchain upgrade to nightly-2025-05-05 by @github-actions in #4060
* Automatic toolchain upgrade to nightly-2025-05-06 by @github-actions in #4061
* Enable target features: x87 and sse2 by @thanhnguyen-aws in #4062
* Fix the bug: Loop contracts are not composable with function contracts  by @thanhnguyen-aws in #3979
* Automatic cargo update to 2025-05-12 by @github-actions in #4066
* Bump tests/perf/s2n-quic from `6aa9975` to `5f323b7` by @dependabot in #4068
* Fix stabilization instructions in RFC intro by @carolynzech in #4067
* Add support for quantifiers by @qinheping in #3993
* Toolchain upgrade to nightly-2025-05-07 by @thanhnguyen-aws in #4070
* Automatic toolchain upgrade to nightly-2025-05-08 by @github-actions in #4071
* Automatic toolchain upgrade to nightly-2025-05-09 by @github-actions in #4072
* Automatic toolchain upgrade to nightly-2025-05-10 by @github-actions in #4073
* Clippy/Stylistic Fixes by @carolynzech in #4074
* Upgrade toolchain to 2025-05-14 by @zhassan-aws in #4076
* Autoharness argument validation: only error on `--quiet` if `--list` was passed by @carolynzech in #4069
* Upgrade Rust toolchain to 2025-05-16 by @zhassan-aws in #4080
* Automatic toolchain upgrade to nightly-2025-05-17 by @github-actions in #4081
* Add setup scripts for Ubuntu 20.04 by @zhassan-aws in #4082
* Automatic toolchain upgrade to nightly-2025-05-18 by @github-actions in #4083
* Automatic cargo update to 2025-05-19 by @github-actions in #4086
* Automatic toolchain upgrade to nightly-2025-05-19 by @github-actions in #4085
* Automatic toolchain upgrade to nightly-2025-05-20 by @github-actions in #4091
* Bump tests/perf/s2n-quic from `5f323b7` to `22434aa` by @dependabot in #4089
* Fix the error that Kani panics when there is no external parameter in quantifier's closure. by @thanhnguyen-aws in #4088
* Update toolchain to 2025-05-22 by @carolynzech in #4098
* Use our toolchain when invoking `cargo metadata` by @carolynzech in #4090
* Automatic toolchain upgrade to nightly-2025-05-23 by @github-actions in #4099
* Automatic toolchain upgrade to nightly-2025-05-24 by @github-actions in #4101
* Automatic toolchain upgrade to nightly-2025-05-25 by @github-actions in #4102
* Fix a bug codegening `SwitchInt`s with only an otherwise branch by @bkirwi in #4095
* Automatic toolchain upgrade to nightly-2025-05-26 by @github-actions in #4104
* Automatic cargo update to 2025-05-26 by @github-actions in #4105
* Bump tests/perf/s2n-quic from `22434aa` to `550afb3` by @dependabot in #4106
* Automatic toolchain upgrade to nightly-2025-05-27 by @github-actions in #4107
* Update `kani::mem` pointer validity documentation by @carolynzech in #4092
* Add support for edition 2018 crates using assert! (Fixes #3717) by @sintemal in #4096
* Automatic toolchain upgrade to nightly-2025-05-28 by @github-actions in #4113
* Automatic toolchain upgrade to nightly-2025-05-29 by @github-actions in #4115
* Automatic toolchain upgrade to nightly-2025-05-30 by @github-actions in #4118
* Handle generic defaults in BoundedArbitrary derives by @zhassan-aws in #4117
* Automatic cargo update to 2025-06-02 by @github-actions in #4121
* Bump tests/perf/s2n-quic from `550afb3` to `8f54b57` by @dependabot in #4122
* Upgrade Rust toolchain to 2025-06-02 by @zhassan-aws in #4123
* Automatic toolchain upgrade to nightly-2025-06-03 by @github-actions in #4125
* Finish deprecating `--enable-unstable`, `--restrict-vtable`, and `--write-json-symtab` by @carolynzech in #4110
* `ty_mangled_name`: only use non-mangled name if `-Zcffi` is enabled. by @carolynzech in #4114
* Improve Help Menu by @carolynzech in #4109
* Start stabilizing `--jobs` and `list`; deprecate default memory checks by @carolynzech in #4108
* Refactor simd_bitmask to reduce the number of iterations by @zhassan-aws in #4129
* Set target features depending on the target architecture by @zhassan-aws in #4127
* Bump some versions suggested by cargo-outdated by @zhassan-aws in #4131
* Improve linking error output for `#[no_std]` crates by @AlexanderPortland in #4126
* Fix the git log command in the toolchain update script by @zhassan-aws in #4139
* Gate quantifiers behind an experimental feature by @thanhnguyen-aws in #4141
* Automatic cargo update to 2025-06-09 by @github-actions in #4145

## New Contributors
* @bkirwi made their first contribution in #4095
* @sintemal made their first contribution in #4096
* @AlexanderPortland made their first contribution in #4126

**Full Changelog**: kani-0.62.0...kani-0.63.0
```


By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.

---------

Co-authored-by: Carolyn Zech <carolynzech@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatic toolchain upgrade fails with argument-list-too-long
2 participants