imyller / meta-nodejs

OpenEmbedded layer for latest Node.js releases
MIT License
79 stars 87 forks source link

npm.bbclass: add a oe_runnpm_build() #7

Closed codyps closed 9 years ago

codyps commented 9 years ago

Allows invocation of npm for the build machine, useful things using gulp for generation of static assets.

Also slightly adjusts the bbnote output to indicate "target" or "build"

imyller commented 9 years ago

I think this..

        local t=$1
        shift
        bbnote $1 ${NPM} ${NPM_ARCHFLAGS} ${NPM_FLAGS} "$@"

Should be like this:

        local t=$1
        shift
        bbnote ${t} ${NPM} ${NPM_ARCHFLAGS} ${NPM_FLAGS} "$@"

as $1 is the first argument of $@ after the shift.

Also, is this split of oe_runnpm and oe_runnpm_build just about difference in bbnote logging? You already have to call the specific function in your custom recipe, so why not just bbnote log there?

codyps commented 9 years ago

You're right, the bbnote usage needs fixing.

The split was to supply a different NPM_ARCHFLAGS & CXX to npm. True, I could just drop it in a custom recipe or class, but it seems like others might also need to have npm build modules for host (ie: native) rather than target in a recipie which isn't native.

imyller commented 9 years ago

This is a good idea with good intentions, but I'm bit worried about the implementation and how self-explanatory it would be to others.

I'd call the build version native and add some more overridability for the native version. I think the correct terms are target and native in OE world?

Other and bit simpler approach is to define NPM_ARCH for example with NPM_ARCH ?= "${TARGET_ARCH}" so that it can be overriden by the recipe with value ${BUILD_ARCH} if needed. Then there can be a single oe_runnpm for target and native npm calls. Obviously we can then add bbnote logging about the NPM_ARCH value used.

Would that serve your purpose?

imyller commented 9 years ago

Hopefully these changes resolve your issue.

You can now set target architecture for each recipe:

NPM_ARCH="${BUILD_ARCH}"

or for each separate oe_runnpm call:

NPM_ARCH="${BUILD_ARCH}" oe_runnpm command

NPM_ARCH defaults to ${TARGET_ARCH}

codyps commented 9 years ago

@imyller thanks, that looks like it will work just fine for me. I'll try it out in the next few days.

codyps commented 9 years ago

Unfortunately, because '${NPM_ARCH}' is expanded by buildbot before the shell is given the script, it doesn't appear that NPM_ARCH="${BUILD_ARCH}" oe_runnpm command works.