--
To unsubscribe from this group and stop receiving emails from it, send an email to gn-dev+un...@chromium.org.
This sort of argument can be applied to many things (e.g., code formatting), and yet some may also fairly argue that having more knobs adds complexity for not an obviously positive value (e.g., `go fmt`, the general pattern of convention-over-configuration).That said, I'll note a couple other things about this specific topic:1) It is true that absolute paths can cause issues for reproducibility when the paths are embedded in other things (e.g., debug symbols). However, they may not cause issues (e.g., invoking a command line script may not care whether the path is relative or absolute), and it's debatable whether having things like the ninja files themselves be hermetic and path-independent is nearly as useful.2) Chromium appears to use the 1-arg format of rebase_path() approximately 5-6% of the time (~100 out of 1800 calls in chromium/src). I suspect nearly all if not all of them could easily be changed to default to path relative to root_build_dir or a 2-arg format. I do think in retrospect having the default be relative to root_build_dir would've been more useful, and I would be fine with a global (potentially breaking) change to that default, or a global (potentially breaking) change to make the second arg be required to be more self-documenting. I wouldn't be all that excited about a project-level switch for this.
I like Brett's idea of changing the default value of new_base to root_build_dir. But I reckon we'd need to put this behavior behind a flag, at least initially, no?
$whatever == get_path_info(path, "abspath")
On Mon, Apr 26, 2021 at 2:52 PM Shai Barack <sha...@google.com> wrote:I like Brett's idea of changing the default value of new_base to root_build_dir. But I reckon we'd need to put this behavior behind a flag, at least initially, no?I think we'd remove the one-arg version completely, roll GN everywhere, and then add it back with the new behavior.
I don't think new cases are getting added at such a rate that it's worth adding a complicated regression feature. It's easy to change the existing cases and it's also easy to check for existing ones with a local GN build. Even if somebody races with you and adds a new one while you're trying to roll, that's fast enough to fix. The cases where the allowlists helped were more complicated changes where somebody might have to actually think about how to fix it.Brett
--
On Mon, Apr 26, 2021 at 2:56 PM Brett Wilson <bre...@chromium.org> wrote:On Mon, Apr 26, 2021 at 2:52 PM Shai Barack <sha...@google.com> wrote:I like Brett's idea of changing the default value of new_base to root_build_dir. But I reckon we'd need to put this behavior behind a flag, at least initially, no?I think we'd remove the one-arg version completely, roll GN everywhere, and then add it back with the new behavior.Could we drop the one-arg version of `rebase_path()`, and then add `rebase_path_from_root()` (or a suitable better name), so that the one-arg usage is clearer in what it's behavior is?