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

Make generation of demo.svg deterministic #625

Merged
merged 2 commits into from
Sep 9, 2019

Conversation

Helcaraxan
Copy link
Contributor

@Helcaraxan Helcaraxan commented Jul 26, 2019

While working on #605 we found out that the PR's CI revealed an unexpected diff in docs/demo.svg. CI on master looked fine as well but I could reproduce the failure nonetheless locally by running make -B docs/demo.svg.

The rootcause is as follows:

  1. The generation of demo.svg depends on a Node tool that is installed via npm install svg-term-cli. Underneath this the npm command performs its own recursive dependency resolution and fetches the latest version (that is compatible with any existing version constraints) of all required direct & indirect dependencies.

  2. The latest CI run on master dates back to 13/14 July and passed correctly. Since then the svgo NPM package, a direct dependency of svg-term-cli, released a new minor version making the most recent version 1.3.0 instead of 1.2.2 which is still compatible with the >=1.0.3 constraint defined by svg-term-cli. The move from 1.2.2 to 1.3.0 apparently introduced a change that resulted in a diff in the generated docs/demo.svg file.

The solution here (and which will help prevent this occurring in the future as well) is to fix the versions of all (in)direct dependencies used to install the svg-term-cli package.

This is achieved by creating a fake NPM package in the tools folder using a package.json with svg-term-cli as a dependency and a package-lock.json file to hardcode the required version for each dependency. This also allows us to run npm ci instead of npm install svg-term-cli which is designed specifically for the automated runs of a CI environment.

This will make the generation of docs/demo.svg deterministic and allows the safe upgrade of any of the required package versions in the future via code-reviewed updates to the package.json and package-lock.json files.

@CLAassistant
Copy link

CLAassistant commented Jul 26, 2019

CLA assistant check
All committers have signed the CLA.

@Helcaraxan Helcaraxan force-pushed the fix/demo-svg-gen branch 3 times, most recently from 62af2cd to c116eaf Compare July 26, 2019 09:47
@@ -1,11 +1,15 @@
sudo: false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing this as it's deprecated according to Travis documentation.

@pierrre
Copy link
Contributor

pierrre commented Aug 5, 2019

Any news from the maintainers ?

@Helcaraxan
Copy link
Contributor Author

Unfortunately not. I can't seem to find any trace of their activity on the project over the past 3 weeks. Probably due to it being the summer holiday season, so quite understandable. 🙂

@jirfag
Copy link
Member

jirfag commented Sep 9, 2019

thank you!

@Helcaraxan
Copy link
Contributor Author

You're welcome. 🙂 Happy to see this getting merged!

@jirfag jirfag merged commit 0b49095 into golangci:master Sep 9, 2019
@ldez ldez added this to the v1.18 milestone Mar 6, 2024
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.

5 participants