Skip to content

Blacklist known bad Erlang releases, fixes #1857#1871

Merged
wohali merged 1 commit intomasterfrom
blacklist-versions
Jan 23, 2019
Merged

Blacklist known bad Erlang releases, fixes #1857#1871
wohali merged 1 commit intomasterfrom
blacklist-versions

Conversation

@wohali
Copy link
Copy Markdown
Member

@wohali wohali commented Jan 22, 2019

Some of the changes to .travis.yml are temporary - once the test run looks correct, a screenshot of the output will be added and the known-bad versions will be removed.

In support of #1870
Closes #1857

@wohali wohali requested a review from nickva January 22, 2019 20:42
@wohali wohali force-pushed the blacklist-versions branch 3 times, most recently from b260550 to d8c8d3e Compare January 22, 2019 20:57
@wohali
Copy link
Copy Markdown
Member Author

wohali commented Jan 22, 2019

So we have a problem. Travis only has 20.3.8, 20.3.8.1 and 20.3.8.5 available for testing. They don't actually have 20.3.8.11 or higher. (Inventory: https://s3.amazonaws.com/travis-otp-releases)

Options:

  • Get Travis admins to provide 20.3.8.18.
  • Detect that we're in a Travis CI run and skip the version check (see nNck's comment below)
  • Force Travis to use kerl to build 20.3.8.11+, which would happen every build. (Big slowdown.)
  • Install 20.3.8.18 from packages.erlang-solutions.com in .travis.yml. (Fiddly .travis.yml work.)
  • Print a warning at compile time instead of a hard halt.
  • Print a warning instead of a hard halt, but only when we match against 20.3.8
  • Don't have Travis test against 20.3.8 anymore, just 19.3 and 21.2.3 (or newer)

@davisp @nickva @janl what do you think?

@nickva
Copy link
Copy Markdown
Contributor

nickva commented Jan 22, 2019

Maybe don't halt on 20.3.8 and print the warning if running in Travis? There is TRAVIS=true environment variable we can check I think. With some comment saying "when/if 20.3.8.11 + becomes available we'll make it halt".

Comment thread rebar.config.script Outdated
@wohali wohali force-pushed the blacklist-versions branch 3 times, most recently from 6d4fd48 to 5f0abfa Compare January 22, 2019 23:10
@wohali wohali changed the title [WIP] Blacklist known bad Erlang releases, fixes #1857 Blacklist known bad Erlang releases, fixes #1857 Jan 22, 2019
@wohali
Copy link
Copy Markdown
Member Author

wohali commented Jan 22, 2019

I opted to wrap the halt call in a check for the TRAVIS envvar. We should be smart enough to not put a bad version number intentionally into .travis.yml.

./configure locally fails on 20.3.8.6, 20.3, and 21.2.
./configure locally passes on 19.3.6.8, 20.3.8.18 and 21.2.3.

Comment thread rebar.config.script Outdated
@wohali wohali force-pushed the blacklist-versions branch from 5f0abfa to 13fee56 Compare January 23, 2019 00:04
@nickva
Copy link
Copy Markdown
Contributor

nickva commented Jan 23, 2019

I think can simplify the test with:

case VerList of
    [20 | _] = V20 when V20 < [20, 3, 8, 11] -> NotSupported(VerString);
    [20 | _] = V20 when V20 >=  [20, 3, 8, 11] -> ok;
    [21, 2] -> NotSupported(VerString);
    [21, 2, N | _] when N < 3 -> NotSupported(VerString);
    _ -> ok
end.

This would take care of 20.2.9.9 type versions. What we are saying there if it's a 20 version, anything less than 20.3.8.11 is no good. Otherwise (>=) it's ok.

Copy link
Copy Markdown
Contributor

@nickva nickva left a comment

Choose a reason for hiding this comment

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

+1

@wohali wohali force-pushed the blacklist-versions branch from 13fee56 to 25d473d Compare January 23, 2019 00:13
@wohali wohali merged commit feaf3da into master Jan 23, 2019
@wohali wohali deleted the blacklist-versions branch January 23, 2019 00:49
janl pushed a commit that referenced this pull request Feb 7, 2019
janl pushed a commit that referenced this pull request Feb 17, 2019
AGrzes pushed a commit to AGrzes/couchdb that referenced this pull request Nov 2, 2019
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.

2 participants