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.

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

Ensure no more binaries are checked in #58

Closed
huonw opened this issue Jun 25, 2014 · 9 comments · Fixed by #124
Closed

Ensure no more binaries are checked in #58

huonw opened this issue Jun 25, 2014 · 9 comments · Fixed by #124

Comments

@huonw
Copy link
Member

huonw commented Jun 25, 2014

Updated description

make test should ensure no binaries are checked in.

Original Description

repo contains a 4MB binary

https://github.com/rust-lang/cargo/blob/master/tests/tests

cargo is still young enough that this could possibly be rebased out, to keep the repository slim. (Just removing it in a normal commit will not remove it from the history, meaning it will hang around in .git forever: this would need some pickaxing/history modification.)

@huonw
Copy link
Member Author

huonw commented Jun 25, 2014

This is 90% of cargo's source size:

$ git ls-files | grep -v libs/ | xargs du -c | sort -n
4       DESIGN/LIBS.md
4       .gitignore
4       .gitmodules
4       LICENSE-MIT
4       Makefile
4       README.md
4       src/bin/cargo-build.rs
4       src/bin/cargo-git-checkout.rs
4       src/bin/cargo-read-manifest.rs
4       src/bin/cargo-rustc.rs
4       src/bin/cargo-verify-project.rs
4       src/cargo/core/dependency.rs
4       src/cargo/core/errors.rs
4       src/cargo/core/mod.rs
4       src/cargo/core/package_id.rs
4       src/cargo/core/registry.rs
4       src/cargo/core/summary.rs
4       src/cargo/ops/cargo_compile.rs
4       src/cargo/ops/cargo_read_manifest.rs
4       src/cargo/ops/mod.rs
4       src/cargo/sources/git/mod.rs
4       src/cargo/sources/mod.rs
4       src/cargo/sources/path.rs
4       src/cargo/util/graph.rs
4       src/cargo/util/important_paths.rs
4       src/cargo/util/mod.rs
4       src/cargo/util/paths.rs
4       src/cargo/util/result.rs
4       tests/fixtures/hello.rs
4       tests/support/paths.rs
4       tests/test_shell.rs
4       tests/tests.rs
4       .travis.check.style.sh
4       .travis.install.deps.sh
4       .travis.yml
4       .vimrc
8       DESIGN/DESIGN.md
8       DESIGN/MANIFEST.md
8       MANIFEST.md
8       src/bin/cargo.rs
8       src/cargo/core/manifest.rs
8       src/cargo/core/package.rs
8       src/cargo/core/resolver.rs
8       src/cargo/core/shell.rs
8       src/cargo/core/source.rs
8       src/cargo/lib.rs
8       src/cargo/ops/cargo_rustc.rs
8       src/cargo/sources/git/source.rs
8       src/cargo/util/config.rs
8       src/cargo/util/errors.rs
8       src/cargo/util/process_builder.rs
8       src/cargo/util/toml.rs
12      LICENSE-APACHE
12      src/cargo/core/version_req.rs
12      src/cargo/sources/git/utils.rs
12      tests/support/mod.rs
12      tests/test_cargo_compile_path_deps.rs
16      tests/test_cargo_compile_git_deps.rs
16      tests/test_cargo_compile.rs
4028    tests/tests
4392    total

(grep -v libs/ to remove the submodules, since the du doesn't handle them nicely. hammer.rs and hamcrest-rust are both <50kb, toml-rs is about 500kb and >400kb of that is tests.)

@steveklabnik
Copy link
Member

That involves re-writing the entire history, which means everyone's checkouts are invalid.

It's easy to run git-filter-branch, that's not the issue.

@wycats
Copy link
Contributor

wycats commented Jun 25, 2014

4MB is kind of bad 😦

I don't like rewriting history, but this is a kind of extreme case.

@utkarshkukreti
Copy link

There's also a 2.3MB file res in the git history, added in 3ac946a and removed later.

https://github.com/rust-lang/cargo/blob/3ac946a2c9f0481165aa29a43113f9c764fe4d9c/res

@alexcrichton alexcrichton changed the title repo contains a 4MB binary Ensure no more binaries are checked in Jun 27, 2014
@alexcrichton
Copy link
Member

I believe that history is quite important, even for a fledgling project such as this. I've updated the title/description to the course of action I believe needs to be taken to resolve this issue.

@huonw
Copy link
Member Author

huonw commented Jun 27, 2014

filter-branch will preserve the rest of the history and just remove the 6+MB of binaries.

@alexcrichton
Copy link
Member

Hm, I may not fully understand what filter-branch is doing then. I ran these two commands:

git filter-branch -f --index-filter 'git rm --cached --ignore-unmatch res' HEAD
git filter-branch -f --index-filter 'git rm --cached --ignore-unmatch tests/tests' HEAD

And the resulting commit differed by 197 commits form the current HEAD. I was under the impression that only the two commits which introduced those files actually had their contents changed, but all other commits changed hashes because they have a different lineage, even if the same contents.

If we could modify just those two commits it would be nice, but is that possible?

@huonw
Copy link
Member Author

huonw commented Jun 27, 2014

Oh, sorry, yeah, I was talking about "preserving history" in a more abstract sense. The commits will change (since info about the lineage is included in the hash, as you say), but the actual contents of each commit would be unchanged (other the ones actually touching res/tests/tests).

@alexcrichton
Copy link
Member

Ah, that makes sense!

alexcrichton added a commit to alexcrichton/cargo that referenced this issue Jul 8, 2014
@bors bors closed this as completed in #124 Jul 8, 2014
alexcrichton added a commit to alexcrichton/cargo that referenced this issue Sep 2, 2014
bors added a commit to alexcrichton/cargo that referenced this issue Sep 2, 2014
ehuss pushed a commit to ehuss/cargo that referenced this issue Nov 19, 2023
58: Don't require clippy r=oli-obk a=Manishearth

This makes it more suitable to use for epoch stuff


We also need better error reporting here :|

(Can this get a release?)

r? @killercup
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 a pull request may close this issue.

5 participants