-
Notifications
You must be signed in to change notification settings - Fork 77
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
Switch npm from buildchroot to schroot #469
Conversation
And remove isar patch for optee-os-tadevkit since it's included now. Signed-off-by: Uladzimir Bely <[email protected]>
This implements a sbuild chroot with 'npm' preinstalled. It is supposed to be used for building packages that inherit npm.bbclass. Signed-off-by: Uladzimir Bely <[email protected]>
Switch from buildchroot to npm-flavored sbuild chroot for preparing and building npm packages. Signed-off-by: Uladzimir Bely <[email protected]>
layers: | ||
meta: | ||
patches: | ||
optee-os-tadevkit: | ||
path: isar-patches/0001-optee-os-Add-package-optee-os-tadevkit.patch |
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.
I would prefer to get that done before this series, and that will not be that simple to move secure boot to the isar scheme.
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.
IOW, #455 needs to come first.
require recipes-devtools/sbuild-chroot/sbuild-chroot-target.bb | ||
|
||
SBUILD_FLAVOR = "npm" | ||
SBUILD_CHROOT_PREINSTALL_EXTRA ?= "npm dpkg-dev" |
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.
As discussed in #433, this will need more. I wonder why you didn't check that MR.
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.
I didn't see this pull-request. Will look later.
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.
Looked a bit at #433. It seems to be related just to switching to derived chroot (with "npm" preintalled), while the patchset I propose is mostly about complete removing of buildchroot from npm.bbclass.
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.
Sure, but that does not mean we should ignore what we discussed there.
@@ -211,7 +167,7 @@ python fetch_npm() { | |||
json_objs = {'dependencies': { npmpn: '' }} | |||
json.dump(json_objs, outfile, indent=2) | |||
|
|||
runcmd(d, "npm ci --global-style --ignore-scripts --verbose", "fetch-tmp") | |||
runcmd(d, "npm ci --global-style --ignore-scripts --verbose") |
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.
Why dropping "fetch-tmp"?
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.
I suppose that is because we are running this in a schroot, no longer in WORKDIR. But then there are more places where this became obsolete. Still looks inconsistent.
Buildroot in isar upstream is gone - are you planning to refresh this, or should we take over, @WiseLord? |
OK, I'm taking over. |
Merged via #493 |
This simplifies the code, allows to use cleaner prepare/build environment for npm packages and drops dependency on buildchroot that is going to be removed from isar soon.