Track mib compilation artifacts #2709
Conversation
Turns out this didn't work! Now it does. The check of edge acted as a workaround that prevented clearing more files than necessary, but when multiple output artifacts existed, it over-deleted them, apparently. So now this is properly checked as a bit of logic and all tests keep passing.
This allows incremental builds with these files.
| false; | ||
| false -> | ||
| %% must be header file or artifact | ||
| digraph:in_edges(G, F) == [] andalso maybe_rm_vertex(G, F), |
There was a problem hiding this comment.
Specifically here, the artifact file was possibly deleted more than required when the previous implementation didn't match on the extension (we pass in [".beam"] for ArtifactExt, or for the MIB compiler, [".hrl", ".bin"]), but the .erl compiler used the on-disk build artifacts to re-fetch the compile opts as a fallback and likely hid the bug.
This might represent a perf gain?
There was a problem hiding this comment.
@max-au this is code you had initially written, I don't know if you still care about that aspect of the workflow or if you've moved elsewhere tooling wise yet.
There was a problem hiding this comment.
We didn't have any *.mib files or anything similar, so I guess I could never notice it. Change itself looks fine here, although it appears to be fragile enough to at least attempt a test...
There was a problem hiding this comment.
Yeah the test is implicitly covered by the MIB test. Without that change, the compiler permanently kept deleting build artifacts' from the DAG and recompiling them, which failed the test case. I'll look at making a more explicit one though.
There was a problem hiding this comment.
That was a good call, turns out there were dangling artifact vertices in the DAG as well (the files were deleted and the edge removed when the source went away, but not the artifact vertex). The new regression test covers that as well.
ferd
force-pushed
the
track-mib-compilation-artifacts
branch
from
661d79c to
c13ed8f
Compare
|
ugh, will fix the sorting issue with the windows paths by doing crappy naming schemes. |
This discovered a bug where dangling artifact files could be left in the DAG and bloat it over time. This was unlikely to be a real problem, but it's nice to be clean.
ferd
force-pushed
the
track-mib-compilation-artifacts
branch
from
c13ed8f to
cb084db
Compare
Track mib compilation artifacts
New features: - [Add --offline option and REBAR_OFFLINE environment variable](#2643) - [Add support for project-local plugins](#2697) - [Add eunit --test flag](#2684) Experimental features for which we promise no backwards compatibility in the near future: - [Experimental vendoring provider](#2689) - [Support plugins in experimental vendor provider](#2702) Other changes: - [Support OTP 23..25 inclusively](#2706) - [Bump Relx to 4.7.0](#2718) - [Use `erlexec` directly in relx helper functions](erlware/relx#902) - [Make rlx_util:parse_vsn parse integer versions](erlware/relx#913) - [fix awk script check_name() in extended_bin](erlware/relx#915) - [avoid crash when overlay is malformed](erlware/relx#916) - [keep attributes when stripping beams](erlware/relx#906) - [Fix {include_erts,true} in Windows releases](erlware/relx#914) - [ensure the erl file is writable before copying dyn_erl to it](erlware/relx#903) - Various tests added - [Properly carry overlay_vars settings for files in relx](#2711) - [Track mib compilation artifacts](#2709) - [Attempt to find apps in git subdirs (sparse checkouts)](#2687) - [Do not discard parameters --system_libs and --include-erts when duplicate values exist](#2695) - [Use default `depth` parameter for SSL](#2690) - [Fix global cache config overriding](#2683) - [Error out on unknown templates in 'new' command](#2676) - [Fix a typo](#2674) - [Bump certifi to 2.9.0](#2673) - [add a debug message in internal dependency fetching code](#2672) - [Use SPDX id for license in template and test](#2668) - [Use default branch for git and git_subdir resources with no revision](#2663) Signed-off-by: Fred Hebert <[email protected]>
This has two parts:
Turns out this didn't work! Now it does. The check of edge acted as a
workaround that prevented clearing more files than necessary, but when
multiple output artifacts existed, it over-deleted them, apparently.
So now this is properly checked as a bit of logic and all tests keep
passing.
This allows incremental builds with these files.