View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0006431 | Taler | exchange | public | 2020-07-16 20:06 | 2021-09-02 18:14 |
| Reporter | Florian Dold | Assigned To | Christian Grothoff | ||
| Priority | normal | Severity | trivial | Reproducibility | have not tried |
| Status | closed | Resolution | fixed | ||
| Product Version | git (master) | ||||
| Target Version | 0.8 | Fixed in Version | 0.8 | ||
| Summary | 0006431: consider committing the generated error codes header | ||||
| Description | Currently 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? | ||||
| Tags | No tags attached. | ||||
|
|
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. |
|
|
(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. |
|
|
Current discussion/compromise: modify build to avoid re-generation of files only via bootstrap, instead make it part of the regular build (Makefile[.am]). |
|
|
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. |
|
|
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. |
|
|
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. |
|
|
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. |
|
|
Ah, got your point. Should be better now. |
|
|
Fix committed to master branch. |
|
exchange: master c8a370d9 2020-07-22 23:27 Details Diff |
make GANA update more compatible with incremental compilation (fixes 0006431) |
Affected Issues 0006431 |
|
| mod - bootstrap | Diff File | ||
| mod - contrib/gana | Diff File | ||
| mod - contrib/gana.sh | Diff File | ||
|
exchange: master d72816cf 2020-07-24 14:43 Details Diff |
fix 0006431 |
Affected Issues 0006431 |
|
| mod - Makefile.am | Diff File | ||
| mod - contrib/gana | Diff File | ||
| add - contrib/gana-update.sh | Diff File | ||
| mod - contrib/gana.sh | Diff File | ||
| 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 |
| 2020-08-19 19:42 | Christian Grothoff | Target Version | 0.8.1 => 0.8 |
| 2021-08-24 16:23 | Christian Grothoff | Status | resolved => closed |
| 2021-09-02 18:13 | Christian Grothoff | Changeset attached | => Taler-exchange master d72816cf |
| 2021-09-02 18:13 | Christian Grothoff | Changeset attached | => Taler-exchange master c8a370d9 |
| 2021-09-02 18:14 | Christian Grothoff | Note Added: 0018247 |
