Remove generated files from source control

[osmaczko]

Problem

Remove the output of make generate from source control. Nowadays, almost every PR produces a ridiculous number of changes because contributors use different versions of the tools responsible for generation. This makes PRs difficult to review since most of the changes are unrelated to the actual issue.

Acceptance Criteria

• [ ] remove all generated files from source control
• [ ] enforce the use of the Nix shell in the Makefile to ensure uniform tool usage

[osmaczko]

@jakubgs I would love to hear your opinion. Is there anything else I should think about?

[jakubgs]

I'm all for not storing generated files in the repo, that is silly. We can always do that at build time.

But the fundamental issue is that we're using varied version of tooling, and this could be easily remedied by using an already defined Nix shell on dev environments: https://github.com/status-im/status-go/blob/develop/shell.nix

We could achieve this the same way we do it in status-mobile, by enforcing use of the Nix shell in the Makefile, like so:

# WARNING: This has to be located right before all the targets.
SHELL := ./nix/scripts/shell.sh

shell: export TARGET ?= default
shell: ##@prepare Enter into a pre-configured shell
ifndef IN_NIX_SHELL
	@ENTER_NIX_SHELL
else
	@echo "${YELLOW}Nix shell is already active$(RESET)"
endif

https://github.com/status-im/status-mobile/blob/ca2b3abb1c3091712fc2d5e4300185394a6fb77f/Makefile#L67-L76

Of course some shells would not use Nix shell, but that can be always overriden using the SHELL variable, for example:

_fix-node-perms: SHELL := /bin/sh
_fix-node-perms: ##@prepare Fix permissions so that directory can be cleaned
	$(shell test -d node_modules && chmod -R 744 node_modules)
	$(shell test -d node_modules.tmp && chmod -R 744 node_modules.tmp)

https://github.com/status-im/status-mobile/blob/ca2b3abb1c3091712fc2d5e4300185394a6fb77f/Makefile#L123-L126

It would be fairly simple to adopt Nix setup from status-mobile to status-go, me and @yakimant can help.

[osmaczko]

Thanks for the quick response. That makes a lot of sense and will keep the CI scripts untouched. I would apply both solutions: remove generated files from the repo and enforce using the Nix shell in the Makefile. Shall I create a separate task for adopting the Nix setup from status-mobile to status-go, and should I assign it to you or @yakimant?

[jakubgs]

A separate issue might make sense. It would be fairly easy for me or Anton to do it, but how about you do it and learn a bit about our Nix setup in the process? I'm happy to explain how this should be done and help debug potential issues.

[osmaczko]

I'm happy to learn. Maybe I'll have time to pick it up next week. I'll ask for explanations then. Thanks!

[osmaczko]

@jakubgs unfortunately, I don't think I will have time for this task anytime soon, as I am already full of higher-priority, protocol-level tasks.

15k+ lines of changes on almost each status-go PR is killing me. Would you be a lifesaver and handle the Nix setup for status-go, please :pray: ? Also, let me know if you'd like me to create a separate task for this mission.

[jakubgs]

That's fine @osmaczko, understandable.

I don't think this is easily doable right now without making devs lives a bit worse in another way, see this issue:

• https://github.com/waku-org/go-waku/issues/877

Unless developers agree that updating the vendor SHA is less annoying than dealing with that massive vendor folder.

I'm thinking @apentori could take this task, as he wants to work more on CI and Nix things.

[jakubgs]

If we don't use flakes, it should be in theory doable to script updating the SHA(have not tested this). And that in turn could possibly be added as a Git hook, to avoid devs having to remember to make the change themselves.

[osmaczko]

That's fine @osmaczko, understandable.

Thank you.

Unless developers agree that updating the vendor SHA is less annoying than dealing with that massive vendor folder.

@jakubgs correct me if I am wrong, but is it necessary to get rid of the vendor folder in order to make nix-shell work?

Updating SHA manually still sounds better than varied dev environments. @cammellos may I ask for your opinion here?

If we don't use flakes, it should be in theory doable to script updating the SHA(have not tested this). And that in turn could possibly be added as a Git hook, to avoid devs having to remember to make the change themselves.

That would be great.

[jakubgs]

My recommendation would be to do this in at least 3 steps:

1. Introduce Nix setup from status-mobile and use it only in CI first, minimal or no Makefile changes.
2. Change the Makefile to use the shell implicitly and adjust targets that don't need a Nix shell, remove obsolete ones.
3. Remove generated files, add the to .gitignore, and make sure they are generated for all types of builds.