View Issue Details

IDProjectCategoryView StatusLast Update
0006431Talerexchangepublic2020-07-24 12:50
ReporterFlorian DoldAssigned ToChristian Grothoff 
PrioritynormalSeveritytrivialReproducibilityhave not tried
Status resolvedResolutionfixed 
Product Versiongit (master) 
Target Version0.8.1Fixed in Version0.8 
Summary0006431: consider committing the generated error codes header
DescriptionCurrently a "git pull" doesn't pick up new error codes, since only "./bootstrap" generates the taler_error_codes.h.

Additionally, we now have an (undocumented) dependency on recutils, which is not really necessary (and recutils is less widely packaged than our other dependencies).

Is there any good reason to not just commit the error codes file?
TagsNo tags attached.

Activities

Christian Grothoff

2020-07-16 21:12

manager   ~0016479

Well, mostly lots of additional unnecessary stuff in Git. recutils is then really a developer-only dependency, and developers are likely to need to frequently add error codes, so that dependency just won't go away.

And everyone else won't be affected. So I actually like the current situation.

Florian Dold

2020-07-20 14:47

manager   ~0016490

(I started to think about how to do this because wallet-core.git also needs some way to handle the generated error codes. So this is more a discussion of how our project handles this type of thing in general, not just for this one file.)

Every time we update error codes, we need a git commit anyway. Either with the contents of the file or a new submodule commit hash. Having the generated source code "inline" in the commit makes it easier to audit the change.

I tend to be against having source files that are auto-generated but not directly committed to the repository:

1. It makes it harder for people to compile the code base
2. It kills "jump to definition" in online code browsers (cgit doesn't have this, but GitHub/GitLab) and local tools, unless you at least bootstrap the repository (
3. It likely makes it harder for static analysis tools that don't run a full build

Furthermore, the current approach totally breaks incremental compilation. Every single change to the error codes requires a full rebuild (due to having to do ./bootstrap), which is annoying during development.

Christian Grothoff

2020-07-22 15:08

manager   ~0016495

Current discussion/compromise: modify build to avoid re-generation of files only via bootstrap, instead make it part of the regular build (Makefile[.am]).

Christian Grothoff

2020-07-22 21:35

manager   ~0016501

I've changed the scripting to minimize the impact of GANA regeneration on incremental compilation:
1) bootstrap still triggers contrib/gana.sh
2) manually running contrib/gana.sh can be used to update 'on the spot' without re-running bootstrap OR configure
3) running contrib/gana.sh is harmless if nothing changed (no change to timestamp, no full recompile!)
4) GANA-internal update was optimized to run a smaller set of the recutils steps if possible.

Florian Dold

2020-07-24 09:40

manager   ~0016511

This doesn't solve the issue. One still needs to remember to do "./contrib/gana.sh" after doing a pull. My idea would've been to add a make rule that automatically re-builds the file.

Or, you know, just add the generated file in git and let the developer that adds an error code figure out these issues, not the poor person/CI who just wants to build the code.

Christian Grothoff

2020-07-24 11:53

manager   ~0016513

Eh, you need to remember to contrib/gana.sh after changing GANA. Not after every pull, right? At least for me it seems to work fine assuming the committer did the GANA change properly.

If 'make' were to automatically pull GANA, that would be terrible if we ever remove an error code from GANA, as then older sources would start to FTBFS, as they would try to pull in a more recent GANA. Also, it must be possible to run 'make' while offline.

Not having the generated file in Git saved me already a ton of work with codespell, as it didn't yell at me for the GANA spelling mistakes multiple times.

Florian Dold

2020-07-24 12:18

manager   ~0016515

I'm not saying that "make" should *pull*, but that make should at least update src/include/taler_error_codes_h if the file in the registry.rec file in the submodule changed because of a "git pull".

My main concern with the current state is that it has lead to about half a dozen issues so far, mostly on the side of Marcello's CI stuff. All because of the gana code generation.

Christian Grothoff

2020-07-24 12:50

manager   ~0016516

Ah, got your point. Should be better now.

Issue History

Date Modified Username Field Change
2020-07-16 20:06 Florian Dold New Issue
2020-07-16 20:06 Florian Dold Status new => assigned
2020-07-16 20:06 Florian Dold Assigned To => Christian Grothoff
2020-07-16 21:12 Christian Grothoff Note Added: 0016479
2020-07-16 21:12 Christian Grothoff Severity minor => trivial
2020-07-16 21:12 Christian Grothoff Status assigned => feedback
2020-07-16 21:12 Christian Grothoff Product Version => git (master)
2020-07-20 14:47 Florian Dold Note Added: 0016490
2020-07-20 14:47 Florian Dold Status feedback => assigned
2020-07-22 15:08 Christian Grothoff Note Added: 0016495
2020-07-22 15:09 Christian Grothoff Target Version => 0.8
2020-07-22 21:35 Christian Grothoff Status assigned => resolved
2020-07-22 21:35 Christian Grothoff Resolution open => fixed
2020-07-22 21:35 Christian Grothoff Fixed in Version => 0.8
2020-07-22 21:35 Christian Grothoff Note Added: 0016501
2020-07-24 09:40 Florian Dold Note Added: 0016511
2020-07-24 09:41 Florian Dold Status resolved => feedback
2020-07-24 09:41 Florian Dold Resolution fixed => reopened
2020-07-24 11:53 Christian Grothoff Note Added: 0016513
2020-07-24 12:01 Christian Grothoff Target Version 0.8 => 0.8.1
2020-07-24 12:02 Christian Grothoff Assigned To Christian Grothoff => Florian Dold
2020-07-24 12:18 Florian Dold Note Added: 0016515
2020-07-24 12:50 Christian Grothoff Assigned To Florian Dold => Christian Grothoff
2020-07-24 12:50 Christian Grothoff Status feedback => resolved
2020-07-24 12:50 Christian Grothoff Note Added: 0016516
2020-07-24 12:50 Christian Grothoff Resolution reopened => fixed