TrueCloudLab/frostfs-node

Fork 27

Make make targets more reproducible #305

New issue

Closed

opened 2023-05-02 16:39:38 +00:00 by fyrchik · 6 comments

fyrchik commented

2023-05-02 16:39:38 +00:00

Owner

We have docker targets, but it could also be beneficial to use same version in other cases:

golangci-lint can produce different warnings in new versions, thus different pre-commit behaviour.
protoc-gen-go is fixed, but protoc itself is different.

We have docker targets, but it could also be beneficial to use same version in other cases: 1. golangci-lint can produce different warnings in new versions, thus different pre-commit behaviour. 2. protoc-gen-go is fixed, but protoc itself is different.

fyrchik added the

discussion

triage

labels 2023-05-02 16:39:38 +00:00

fyrchik commented

2023-05-02 16:53:12 +00:00

Author

Owner

So the proposal is following:

Define make bootstrap target which install necessary dependencies in bin or bin/build/. The path can overridden, but it would be nice to install locally by default.
Make other targets use freshly installed binaries by default (allow to easily override).
It would be nice, to avoid installing the linter each time it is run.

Does it look sane? @realloc @dstepanov-yadro

So the proposal is following: 1. Define make `bootstrap` target which install necessary dependencies in `bin` or `bin/build/`. The path can overridden, but it would be nice to install locally by default. 2. Make other targets use freshly installed binaries by default (allow to easily override). 3. It would be nice, to avoid installing the linter each time it is run. Does it look sane? @realloc @dstepanov-yadro

fyrchik commented

2023-05-02 16:57:15 +00:00

Author

Owner

Also, do we have any problems with using docker/* in pre-commit when run locally?

Also, do we have any problems with using `docker/*` in pre-commit when run locally?

ale64bit commented

2023-05-04 10:20:58 +00:00

Member

can't we just run all linters and precommit checks in the CI/cloud pipeline and let people sort out their local config/setup individually?

fyrchik commented

2023-05-04 10:25:14 +00:00

Author

Owner

@realloc please, could you describe your thoughts.

realloc was assigned by fyrchik

2023-05-04 10:25:20 +00:00

realloc commented

2023-05-05 12:19:52 +00:00

Owner

Summarizing, we have few approaches for this problem.

Having a pinned version of tools like protoc in a local bin directory, like we do for hugo in https://git.frostfs.info/TrueCloudLab/frostfs.info/src/branch/master/Makefile#L11

This may cause problems for non-linux users or complicate Makefile with multi-OS support. This also doesn't solve problems with wrong versions of go or make.

Use docker images to wrap make targets, like we do in https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/Makefile#L143

This works well even on Windows. However, we are out of sync between image templates in .docker directory and what we declare in Makefile.

I'd suggest using a combined method when we set desired tool version in Makefile and call it inside container when build a wrapping docker image locally. It would have a desired Go version and all other tools as declared. When run on a normal system it would install what needed on a host. Combined with tools like g/gvm it would help to create isolated environments.

In this case CI should be using the docker image to build and run tests, while developers will have options to use same environment or change for experimental purposes.

Summarizing, we have few approaches for this problem. 1. Having a pinned version of tools like `protoc` in a local `bin` directory, like we do for `hugo` in https://git.frostfs.info/TrueCloudLab/frostfs.info/src/branch/master/Makefile#L11 This may cause problems for non-linux users or complicate Makefile with multi-OS support. This also doesn't solve problems with wrong versions of `go` or `make`. 2. Use docker images to wrap make targets, like we do in https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/Makefile#L143 This works well even on Windows. However, we are out of sync between image templates in `.docker` directory and what we declare in Makefile. I'd suggest using a combined method when we set desired tool version in `Makefile` and call it inside container when build a wrapping docker image locally. It would have a desired Go version and all other tools as declared. When run on a normal system it would install what needed on a host. Combined with tools like g/gvm it would help to create isolated environments. In this case CI should be using the docker image to build and run tests, while developers will have options to use same environment or change for experimental purposes.

fyrchik added this to the v0.38.0 milestone 2023-05-18 08:31:25 +00:00

realloc added the

Infrastructure

label 2023-07-19 14:11:45 +00:00

realloc removed the

triage

label 2023-07-19 14:12:45 +00:00

realloc removed their assignment 2023-07-19 14:13:02 +00:00

fyrchik commented

2024-02-12 06:30:39 +00:00

Author

Owner