make grc use correct config paths #7350
Conversation
marcusmueller
force-pushed
the
make_grc_use_correct_config_paths
branch
from
51485ba
to
5a73405
Compare
| @@ -53,6 +53,7 @@ class Paths(object): | |||
| TOP_BLOCK_FILE_MODE = HIER_BLOCK_FILE_MODE | stat.S_IXUSR | stat.S_IXGRP | |||
|
|
|||
| # Setup paths | |||
| # FIXME: this should use ..main.get_config_directory() and get_state_directory(), probably. | |||
There was a problem hiding this comment.
@haakov I wasn't sure what to do with the commented-out code. Do we get to keep it or should we clean it up?
There was a problem hiding this comment.
I kind of want to get rid of this file. We have Config.py, we have Constants.py, and we have Platform.py in the same directory, and I'm not quite sure what goes where. Maybe we can move the contents of this file to the others?
There was a problem hiding this comment.
OK, then let's do that, it's OK enough an effort to be done quickly; but let's keep that out of this PR, then. I guess that especially means that the commented out code on Lines 57ff will just be removed.
marcusmueller
force-pushed
the
make_grc_use_correct_config_paths
branch
from
5a73405
to
836257d
Compare
| @@ -133,7 +133,12 @@ def build_library(self, path=None): | |||
| self.cpp_connection_templates.clear() | |||
| self._block_categories.clear() | |||
|
|
|||
| with Cache(Constants.CACHE_FILE, version=self.config.version) as cache: | |||
| try: | |||
There was a problem hiding this comment.
my mental model was that GRC is something we perspectively want to move out of the GR tree and make less tightly version-coupled.
In the wake of that, depending on the very recent addition of gr.paths (or gr.paths.persistent) is still sensible (because we're still having GRC in-tree), but falling back gracefully in this place makes it easier to use "new" GRC with "old" GRC in the future.
|
I guess that's a general comment ... why are we assuming that an import from our own package could fail? |
|
Not for this PR, but a wiki writeup of all the "if here", nope, "look there", hmm, "maybe over here", "copy to new place", etc., logic recently added to GRC would be good. |
|
Since caching is just for startup performance, is there really a need to make that part backward compatible? Even switching back and forth between two GRC versions will just cause two caches to be kept up to date. |
You're right! |
marcusmueller
force-pushed
the
make_grc_use_correct_config_paths
branch
from
ad49349
to
eb2cc2a
Compare
Thinking about this some more: you're right, no need to be backwards-compatible (which is great for backporting, I guess!), but we still shouldn't crash if we are faced with the old API only. So, I don't think there's anything to change here :) |
Esp. to be used for GRC-generated python etc. Signed-off-by: Marcus Müller <mmueller@gnuradio.org>
Signed-off-by: Marcus Müller <mmueller@gnuradio.org>
marcusmueller
force-pushed
the
make_grc_use_correct_config_paths
branch
from
eb2cc2a
to
1f593b4
Compare
|
squashed |
|
@willcode ping! |
Description
As discussed in GRC Development,
it would be the right thing if GRC configuration ends up in the same tree as
GNU Radio itself stores configuration. (And we're now doing
XDG_CONFIG_HOME,unless that doesn't exist, but the legacy
.gnuradiodoes.)Same for generated hier blocks: they should live in a standard persistent state
directory. And for caches from the example browser and block lib, use the
appropriate cache dir.
Luckily, GNU Radio offers these paths by now;
gr.pathsis your friend. Justneed to use them.
We're also including fallbacks, should importing that fail for whatever reason.
Related Issues
Closes #7337
Which blocks/areas does this affect?
GRC Qt, GTK, converter
We add (no API breakage) a
persistentgetter tosys_pathsand export it to python.Checklist