feat(cli): lb4 example [<example-name>] #848

bajtos · 2018-01-09T15:00:16Z

Implement a new CLI command for cloning an example project from our monorepo.

Usage:
  lb4 example [options] [<example-name>]

Arguments:
  example-name  # Name of the example to clone  Type: String  Required: false

Available examples:
  codehub: A GitHub-like application we used to use to model LB4 API.

The command:

creates a local directory called loopback4-example-${name},
downloads the current master branch as a .tar.gz file from GitHub in a streaming mode,
and extracts all files from packages/example-${name} to this new directory.

See #836

Checklist

npm test passes on your machine
New tests added or existing tests modified to cover all changes
Code conforms with the style guide
Related API Documentation was updated
Affected artifact templates in packages/cli were updated
Affected example projects in packages/example-* were updated (not applicable)

bajtos · 2018-01-09T15:22:55Z

Re-posting from our internal chat.

Could we / should we use sparse-checkout as described in this answer? https://stackoverflow.com/a/13738951

Well, I don't want to introduce dependency on git, I have vague memories that setting up git is problematic on Windows. IIRC, there is no native version of git on windows, the most popular option uses Cygwin under the hood. One of the reasons is that many git commands are (or used to be) implemented as Bash scripts.

Another reason why I prefer to download a ZIP bundle is that cloning a git repository downloads a full history with all revisions of all files, which is much more data than just the latest content in the master branch.

virkt25 · 2018-01-09T15:53:05Z

packages/cli/generators/example/clone-example.js

+
+    function extractEntry(entry, zipFile, next) {
+      // Skip optional directory entries
+      if (entry.fileName.endsWith('/')) return next();


path.sep constant should be used for windows compatibility.

IIRC, zip entry file names always use /'.

Good catch. According to yauzl's documentation, directory entries in ZIP end with / character - see https://www.npmjs.com/package/yauzl#usage:

if (/\/$/.test(entry.fileName)) { // Directory file names end with '/'. // Note that entires for directories themselves are optional. // An entry's fileName implicitly requires its parent directories to exist. zipfile.readEntry(); }

See also ZIP format specification, section 4.4.17.1:

The name of the file, with optional relative path.
The path stored MUST not contain a drive or
device letter, or a leading slash. All slashes
MUST be forward slashes '/' as opposed to
backwards slashes '' for compatibility with Amiga
and UNIX file systems etc. If input came from standard
input, there is no file name field.

raymondfeng · 2018-01-09T16:40:37Z

Can we avoid the temporary file by piping the http response into gunzip and extract? See an example at https://github.com/strongloop/loopback-oracle-installer/blob/master/lib/download.js#L103.

raymondfeng · 2018-01-09T16:52:33Z

packages/cli/bin/cli.js

  path.join(__dirname, '../generators/controller'),
  'loopback4:controller'
 );
+env.register(


Out of scope for the PR, but we probably should refactor the list of generators into a separate file, such as:

generators.js

module.exports = { app: path.join(__dirname, '../generators/app'), // Path controller: path.join(__dirname, '../generators/controller'), // Path another: AnotherGenerator, // Generator class }

Ideally, I'd like us to discover all generators automatically by scanning generators directory. I believe Yeoman is already doing that (or was doing that in earlier versions) for projects that are following prescribed project layout (see our generator-loopback for an example).

I'm not sure if we really want to use Yeoman discovery, which is one of areas that caused confusions in the past.

I agree we probably don't want to use Yeoman discovery, I'd like us to write our own to follow the conventions we like most. But yeah, it's out of scope of this pull request.

bajtos · 2018-01-09T17:03:13Z

Can we avoid the temporary file by piping the http response into gunzip and extract? See an example at https://github.com/strongloop/loopback-oracle-installer/blob/master/lib/download.js#L103.

Unfortunately not. See https://www.npmjs.com/package/yauzl#no-streaming-unzip-api

Due to the design of the .zip file format, it's impossible to interpret a .zip file from start to finish (such as from a readable stream) without sacrificing correctness. The Central Directory, which is the authority on the contents of the .zip file, is at the end of a .zip file, not the beginning. A streaming API would need to either buffer the entire .zip file to get to the Central Directory before interpreting anything (defeating the purpose of a streaming interface), or rely on the Local File Headers which are interspersed through the .zip file. However, the Local File Headers are explicitly denounced in the spec as being unreliable copies of the Central Directory, so trusting them would be a violation of the spec.

Any library that offers a streaming unzip API must make one of the above two compromises, which makes the library either dishonest or nonconformant (usually the latter). This library insists on correctness and adherence to the spec, and so does not offer a streaming API.

(etc.)

raymondfeng · 2018-01-09T19:31:43Z

I think we should simply use tarball to enable streaming: https://codeload.github.com/strongloop/loopback-next/legacy.tar.gz/master (redirected from https://github.com/strongloop/loopback-next/tarball/master)

virkt25 · 2018-01-09T21:48:23Z

I wonder if git is still problematic on Windows but with version 1.9, sparse checkout can be combined with a shallow clone (to drop history) based on the stackoverflow answer below on that thread. Direct link (different than above): https://stackoverflow.com/a/28039894

If git isn't an issue on windows still this approach might be simpler.

bajtos · 2018-01-11T09:31:37Z

I think we should simply use tarball to enable streaming: https://codeload.github.com/strongloop/loopback-next/legacy.tar.gz/master (redirected from https://github.com/strongloop/loopback-next/tarball/master)

Nice! I didn't know GitHub offers tar.gz downloads too. I'll look into this.

I wonder if git is still problematic on Windows but with version 1.9, sparse checkout can be combined with a shallow clone (to drop history) based on the stackoverflow answer below on that thread. Direct link (different than above): https://stackoverflow.com/a/28039894

If git isn't an issue on windows still this approach might be simpler.

Even if git was easy to install on Windows, I would be still reluctant to depend on it. Here are my concerns:

What if the developer has an older version of git, i.e. pre-1.9?
What if the developer does not have git installed at all, for example because they are a newbie just trying LoopBack out, or because they are using a different SCM tool like Perforce or Mercurial?

The code to handle zip/tgz download and extraction may be slightly more complex to write. My expectation is that this will be "write once, never touch again" case and the extra complexity is more than worth the better user experience it gives us.

bajtos · 2018-01-11T11:39:26Z

Woot! I have reworked the code to download .tar.gz in streaming mode - see 52704fc, I love how much simpler the new implementation is ✌️

@virkt25 @raymondfeng LGTY now?

bajtos · 2018-01-12T08:05:06Z

Rebased on top of the current master.

Implement a new CLI command for cloning an example project from our monorepo. Usage: lb4 example [options] [<example-name>] Arguments: example-name # Name of the example to clone Type: String Required: false Available examples: codehub: A GitHub-like application we used to use to model LB4 API. The command downloads current master branch as a ZIP file from GitHub to a temp file, creates a local directory called `loopback4-example-${name}` and extracts all files from `packages/example-${name}` to this new directory.

virkt25

LGTM

bajtos self-assigned this Jan 9, 2018

bajtos requested review from kjdelisle, raymondfeng and virkt25 January 9, 2018 15:00

bajtos requested a review from shimks as a code owner January 9, 2018 15:00

bajtos mentioned this pull request Jan 9, 2018

Bring examples back into the monorepo #836

Closed

12 tasks

virkt25 reviewed Jan 9, 2018

View reviewed changes

raymondfeng reviewed Jan 9, 2018

View reviewed changes

bajtos requested review from raymondfeng and virkt25 January 11, 2018 11:40

bajtos force-pushed the cli/lb4-example branch from 52704fc to b32271e Compare January 12, 2018 08:05

bajtos force-pushed the cli/lb4-example branch from b32271e to 929af07 Compare January 12, 2018 08:35

virkt25 approved these changes Jan 12, 2018

View reviewed changes

bajtos mentioned this pull request Jan 12, 2018

feat(cli): generate controller with CRUD methods #849

Merged

6 tasks

raymondfeng approved these changes Jan 12, 2018

View reviewed changes

bajtos merged commit 4286c0d into master Jan 15, 2018

bajtos deleted the cli/lb4-example branch January 15, 2018 09:14

bajtos mentioned this pull request Feb 8, 2018

lb4: describe "lb4 example" command loopbackio/loopback.io#614

Merged

feat(cli): lb4 example [<example-name>] #848

feat(cli): lb4 example [<example-name>] #848

Uh oh!

Conversation

bajtos commented Jan 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

bajtos commented Jan 9, 2018

Uh oh!

virkt25 Jan 9, 2018

Choose a reason for hiding this comment

Uh oh!

raymondfeng Jan 9, 2018

Choose a reason for hiding this comment

Uh oh!

bajtos Jan 9, 2018

Choose a reason for hiding this comment

Uh oh!

raymondfeng commented Jan 9, 2018

Uh oh!

raymondfeng Jan 9, 2018

Choose a reason for hiding this comment

Uh oh!

bajtos Jan 9, 2018

Choose a reason for hiding this comment

Uh oh!

raymondfeng Jan 9, 2018

Choose a reason for hiding this comment

Uh oh!

bajtos Jan 11, 2018

Choose a reason for hiding this comment

Uh oh!

bajtos commented Jan 9, 2018

Uh oh!

raymondfeng commented Jan 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

virkt25 commented Jan 9, 2018

Uh oh!

bajtos commented Jan 11, 2018

Uh oh!

bajtos commented Jan 11, 2018

Uh oh!

bajtos commented Jan 12, 2018

Uh oh!

virkt25 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bajtos commented Jan 9, 2018 •

edited

Loading

raymondfeng commented Jan 9, 2018 •

edited

Loading