-
Notifications
You must be signed in to change notification settings - Fork 284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor postinstall.ts so go utilities are built at install time #5133
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ericpromislow
force-pushed
the
5128-refactor-postinstall
branch
from
July 10, 2023 19:02
be10520
to
9d90827
Compare
} | ||
// If rdctl hasn't been compiled yet, we can't use it to get paths. | ||
// Hardwire the paths for scripts (like postinstall, whcih builds rdctl). |
Check failure
Code scanning / check-spelling
Unrecognized Spelling
[whcih](#security-tab) is not a recognized word. \(unrecognized-spelling\)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In draft mode because bats
tests were failing on an unknown version...
ericpromislow
force-pushed
the
5128-refactor-postinstall
branch
2 times, most recently
from
July 10, 2023 19:21
71ec012
to
c7c1119
Compare
ericpromislow
force-pushed
the
5128-refactor-postinstall
branch
from
July 11, 2023 19:28
834a68a
to
3f4561d
Compare
This is so the renderer process can import definitions and not worry about executing any code. In particular, importing `utils/paths.ts` runs the code that sets up the paths and this code can't be run in the renderer process. Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
…n error. Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
…h no other dependencies. The problem is that some of the postinstall code was loading the logger, which was loading `utils/paths.ts`, which depended on finding a working `rdctl`. The first part of the solution is to build `rdctl` earlier and install it in a location where the `paths.ts` code expects to find it. The second part is to break up `postinstall.ts` into two parts, and use the simple-spawner to execute each part, so we don't hit the logger or the paths.ts module until we're ready to. Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
This is because `spawnFile` depends on `paths.ts` to get logs, and in `scripts/` files we can't count on having `rdctl` for the `getPaths` routine. Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
And drop the legacy-integration stuff. Also we can now happily import CURRENT_SETTINGS_VERSION from a defn/constant-only settings.ts. Signed-off-by: Eric Promislow <epromislow@suse.com>
ericpromislow
force-pushed
the
5128-refactor-postinstall
branch
from
July 13, 2023 20:48
c686cdb
to
a5e46f8
Compare
ericpromislow
changed the title
5128 refactor postinstall
Refactor postinstall.ts so go utilities are built at install time
Jul 14, 2023
This branch got mangled during rebase. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #5128
This is required to replace the old node
paths.ts
code with code that relies onrdctl
without keeping a redundant fallback for whenrdctl
isn't found.