chore: fix CLI packaging snapshot relative directory #1713
Merged
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
The current CLI releases are broken. Seems that `pkg` creates the application snapshot based on the current working directory, so at the moment, the snapshot gets created based on the root of the project, rather than based on the dist/Etcher-cli-* directories, causing the native add-ons to not be resolved correctly. Fixes: #1706 Change-Type: patch Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
lurch
reviewed
Sep 4, 2017
| @@ -107,7 +107,7 @@ TARGET_ARCH_ELECTRON_BUILDER = $(shell ./scripts/build/architecture-convert.sh - | |||
| PLATFORM_PKG = $(shell ./scripts/build/platform-convert.sh -r $(PLATFORM) -t pkg) | |||
| ENTRY_POINT_CLI = lib/cli/etcher.js | |||
| ETCHER_CLI_BINARY = $(APPLICATION_NAME_LOWERCASE) | |||
| ifeq ($(TARGET_PLATFORM),win32) | |||
| ifeq ($(PLATFORM),win32) | |||
| @@ -228,7 +228,7 @@ $(BUILD_DIRECTORY)/$(APPLICATION_NAME)-cli-$(APPLICATION_VERSION)-$(PLATFORM)-$( | |||
| $(BUILD_DIRECTORY)/$(APPLICATION_NAME)-cli-$(APPLICATION_VERSION)-$(PLATFORM)-$(TARGET_ARCH)-app \ | |||
| | $(BUILD_DIRECTORY) | |||
| mkdir $@ | |||
| $(NPX) pkg --output $@/$(ETCHER_CLI_BINARY) -t node6-$(PLATFORM_PKG)-$(TARGET_ARCH) $</$(ENTRY_POINT_CLI) | |||
| cd $< && ../../$(NPX) pkg --output ../../$@/$(ETCHER_CLI_BINARY) -t node6-$(PLATFORM_PKG)-$(TARGET_ARCH) $(ENTRY_POINT_CLI) | |||
There was a problem hiding this comment.
This looks a little bit ugly - I wonder if it'd be less ugly to have a pkg-wrapper script, that could 'hide' this ugliness from the Makefile?
Would it be also worth adding an issue to the pkg repo, suggesting they add an additional command-line parameter, that would then make pkg 'natively' do what you want (without this wrapper script)?
There was a problem hiding this comment.
See vercel/pkg#231. I'll merge this so we can release (we're planning a release very soon), but hopefully pkg fixes the issue soon, and we can update this.
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.
The current CLI releases are broken. Seems that
pkgcreates theapplication snapshot based on the current working directory, so at the
moment, the snapshot gets created based on the root of the project,
rather than based on the dist/Etcher-cli-* directories, causing the
native add-ons to not be resolved correctly.
Fixes: #1706
Change-Type: patch
Signed-off-by: Juan Cruz Viotti jv@jviotti.com