Skip to content

Conversation

ewgenius
Copy link
Contributor

@ewgenius ewgenius commented Jan 31, 2020

We did PR #541 on stable branch, moving this changes back to master

Evgenii Khramkov and others added 30 commits January 30, 2020 11:11
Assert that 100,000 lines (instead of 10,000 lines) can complete in 2 seconds.
The inline Ruby wanted versions like 2.3.4, but it was getting int-mapped
versions like 2003004000.  That broke the logic in the inline Ruby
completely, so everything was treated as having a major version wayyyy
above 3, hence "modern".

This is why we unit-test.
It's not installed on either Mac or Linux CI boxes
This reverts commit f1bb5af.

DRY.  Not a big deal either way.
This reverts commit 7411333.

We don't need coreutils.  I used $PWD to eliminate the need for
`realpath`, and I've implemented `timeout` for OSX.  If the Linux CI is
missing `timeout`, I'll push another commit to let it use the OSX
implementation.
We can't rely on the customer's admin box (which isn't the appliance)
having any actual programming languages on it.  But we can use sed, and
sed is good enough to simulate dirname with a single fork+exec.

This commit also fixes future versions: 2.20.x, 3.x, etc.  All future
versions will be >= 2.19.3, so we just combine the 2.19 check with the
future-versions check.
The shell `version` function treats garbage as equivalent to 0.0.0, rather
than throwing exceptions and exiting with an error code, as the Ruby
implementation did.  So roll the invalid-input tests into the
use-the-old-version test.
The data we're sorting has clusters of duplicates in the input, because
`dirname` reduces all repos in the same network (i.e., forks) to the same
network path.  Running `uniq` before `sort` eliminates those duplicates,
which means `sort` requires less CPU and RAM to do its thing.

We still need `uniq` on the output end, because there's no guarantee that
all duplicates in the input are clustered.

I've run tests, and the cost of `uniq` is small enough that it does no
harm if the input has no duplicates at all.
We brought back coreutils, which provides it.
@thejandroman thejandroman mentioned this pull request Feb 18, 2020
@cainejette cainejette merged commit 4ff376b into master Feb 19, 2020
@ewgenius ewgenius deleted the merge-stable-to-master branch February 19, 2020 19:05
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.

4 participants