Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

[WIP] Lockfile checksum #6448

Closed
wants to merge 10 commits into from
Closed

Conversation

agrim123
Copy link
Contributor

@agrim123 agrim123 commented Mar 19, 2018

What was the end-user problem that led to this PR?

The main aim is to reduce Gemfile resolve time if checksum remains same.

What was your diagnosis of the problem?

Add a section CHECKSUMS to lockfile that will store the checksum and will be used to match for future.
I continued the work done by @segiddins.

What is your fix for the problem, implemented in this PR?

The main approach was to store a checksum of current gemfile under CHECKSUMS sections in Gemfile.lock

In continuation of #5584.

@agrim123 agrim123 force-pushed the lockfile-checksum branch 4 times, most recently from 470d7de to 551e8f6 Compare March 19, 2018 20:20
@agrim123
Copy link
Contributor Author

agrim123 commented Mar 21, 2018

@segiddins I am not able to test static gemfile option, do not know how to call it from Gemfile like you said. Can you please help me here? And how shall I proceed with this PR?
And need some help in current failing tests!

@segiddins
Copy link
Member

_static_gemfile! in the DSL

@simi
Copy link
Member

simi commented Mar 22, 2018

Hello! I really like this feature.

I went thru code and I have 2 initial questions. Sorry if they were answered already, but I can't find any previous discussion related to those questions.

  1. Why checksum can't be stored in file next to lock files?
  2. I see md5 is used to digest lock. Can't something different (ex. SHA1) be used since md5 is blocked on FIPS enabled systems? There were some issues with md5 and FIPS before. I know there's md5_available? helper used to avoid problems. But do we really need to introduce new features with md5 and disable them for FIPS enabled systems?

@segiddins
Copy link
Member

MD5 is quicker

@agrim123 agrim123 force-pushed the lockfile-checksum branch 2 times, most recently from d240a2a to 400e3d3 Compare April 7, 2018 17:23
@agrim123 agrim123 force-pushed the lockfile-checksum branch 4 times, most recently from 8bfab55 to 18374bc Compare April 11, 2018 18:05
@agrim123
Copy link
Contributor Author

agrim123 commented Apr 14, 2018

@segiddins
This test block is failing

context "with a current version" do
  let(:base_version) { Gem::Version.create(Bundler::VERSION) }

  it "returns an empty array" do
    expect(subject).to eq([])
  end
end

We have added sections in 2.0 version. And when we are testing with BUNDLER_SPEC_SUB_VERSION=1.98 this fails. How do I counter this?

@segiddins
Copy link
Member

The lockfile parser hash of known sections would need an update

@agrim123
Copy link
Contributor Author

I didn't quite understand how to proceed. Should we not change BUNDLER_SPEC_SUB_VERSION for test?

@hsbt hsbt closed this Apr 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants