Feature/build a package by sam-github · Pull Request #1 · strongloop/strong-build

sam-github · 2014-05-23T21:55:58Z

See TODO.txt for tracking of WIP (replaces the old gist https://gist.github.com/sam-github/8acbde690ef6b5554e91).

I think this does all the mandatory features, remaining work before release is:

the git-related manipulations (will do in another PR)
talk to @ritch and get his help testing the approach against a significant example repo, such as strongloop/loopback-example-full-stack (I had some difficulties with install and run, probably I'm doing it wrong).

Note strong-cli is removed as a dependency, pending a resolution of slc example not actually being installable due to lb versioning.

`slc example` has been fixed so it now can be `npm install`ed, so use it as a dev dependency.

Current build directory may be temporary, get git cleaned, and the pack output might itself get packed. Avoid this mess by putting the pack file one directory up.

sam-github · 2014-05-27T04:30:09Z

I added the 'onto' git command to this PR.

Remaining work is only the committing of build materials.

bajtos · 2014-05-27T06:36:14Z

Why vasync instead of the more common async? No need to change your code, I am just curious.

I noticed it in the projects by the author of posix-getopt, and the "debuggable internal state" appealled to me. Its API is a bit more clunky, IMO, and I'm not using that particular feature, I confess, because this series is pretty trivial and so far has needed no debugging. However, when using async's queue during strong-mq development, I basically had to wrap every interaction with the queue in a debug layer to track state transitions, so became painfully aware of the difficulty in observing progress of async (or more to the point, lack of progress when things aren't working).

bajtos · 2014-05-27T06:49:49Z

// Build output won't get packed if it is .npmignored (a configuration
// error, don't .npmignore your build output) or if there is no .npmignore,
// if it is .gitignored (as they should be). So, create an empty .npmignore
// if there is a .gitignore but not a .npmignore so build products are
// packed.

I am worried this may not work well. .gitignore usually contains other local files that should be excluded from the npm package, e.g. .idea (WebStorm configuration), *.sublime*, *.log, code-coverage and style-check artifacts, etc.

I am not sure if there is a solution how to detect which .gitignore entries apply to npm and which don't. Perhaps use special comments to mark which lines to apply?

I am proposing to implement one of the following:

Keep the current code (create empty .npmignore). Print a warning informing the user that all files will be included in the archive, the user should check the archive created and write his/her own .npmignore file if he/she is not happy with the result.
Don't create any .npmignore file. Print a warning informing the user that build artefacts matched by .gitignore are not included in the archive, the user should check the archive created and write his/her own .npmignore file if he/she is not happy with the result.

bajtos · 2014-05-27T06:56:30Z

Other than the comments above, the patch LGTM so far.

sam-github · 2014-05-27T17:01:48Z

Re: touching of .npmignore.

Does npm read "all" the .gitignore configuration sources? Because personal tools such as editor files shouldn't be in a project's .gitignore, I wonder if npm deals with that at all.

Anyhow, I believe presence of a .gitignore without a .npmignore is almost always a problem, so I should warn about it.
I could warn, and then continue, or warn and create a .npmignore. I'm not sure which is better.

My thinking was: If you always build after doing a 'git clean -xd' (advisable), then you will never have anything in your working tree other than the output of the build, so touching .npmignore is the right thing.

I could go the other way, and not touch it, and just warn? Or just warn if there is a .gyp file, or an npm 'build' script? But I don't want to go too far down the path of magically trying to guess the developer's intention.

So, summary:

I will add a warning about presence of a .gitignore without an .npmignore
Should I remove the auto-creation of a .npmignore? @rmg, you want to weigh in?

rmg · 2014-05-27T19:34:18Z

I don't think .gitignore or .npmignore should be considered for this tool. Those files are for informing about source control and published package contents respectively, and this tool is doing neither. I don't want to create a new .deployignore file, but I'm not sure the semantics of the existing ignore files can be reconciled with what is actually needed for this use case.

ritch · 2014-05-27T21:31:25Z

Is this at a point where I can try it out on the full stack example?

sam-github · 2014-05-27T22:19:54Z

@rmg, recall that the packing is done with npm pack. Or (as you and @bajtos requested), the output at least is compatible with that used by npm pack/publish/install. So, .npmignore is pretty relevant, IMHO. And I wouldn't be doing any rewrites of the bundledDependencies property if npm pack wasn't being used.

@ritch Yes. Ping me if you need help, I had trouble, but I didn't know the code as well as you.

sam-github · 2014-05-27T23:10:06Z

So the thing about having a .gitignore and no .npmignore, is the entire point of this script is to pack build products... and build products are always in .gitignore, so the pack will allways be broken, it won't include build products.

If we create a too permissive .npmignore, we package stuff we don't need... but the pack still works.

So I added docs and a warning message. I think working but extra garbage is better than not working.

A missing .npmignore when .gitignore is present, or both bundleDependencies and bundledDependencies in package.json are problematic.

Checking node_modules to avoid optional deps that were not installed is too unpredictable, and always fails if done without first doing an `npm install`. We now do the predictable thing, bundle deps and optional deps. If this isn't sufficient, the user can provide their own bundledDependencies specification in the package.json.

sam-github · 2014-05-28T04:31:08Z

I've manually tested this with sls-sample-app, but would still like to test with the loopback fullstack app. Will work with @ritch on that.

bajtos · 2014-05-28T08:47:41Z

So I added docs and a warning message. I think working but extra garbage is better than not working.

Agreed.

sam-github · 2014-05-29T20:09:34Z

Btw, manually tested with full stack loopback example, looks like approach is workable, see

strongloop/loopback-example-offline-sync#8 and strongloop/loopback-example-offline-sync#9

rmg · 2014-05-29T20:17:43Z

LGTM.

sam-github added 14 commits May 23, 2014 14:24

License and contribution agreement

2fc17b4

Add package.json and slb stub

b6e34c2

Add tap test script

f65da5d

Implement npm install without build scripts

45468e0

Note strong-cli is removed as a dependency, pending a resolution of slc example not actually being installable due to lb versioning.

Implement usage and version options

dfa9fda

Implement running of custom package build script

5ea2f53

Use option parser, and test option parsing

ed8a7df

Refactor build test setup so it can be reused

b64f480

Implement --scripts, to build binary addons

36f190c

Implement --pack, to create an archive

8211314

Implement --bundle, so dependencies are packed

a3f4fca

test: use strong-cli as a dev dependency

8123af7

`slc example` has been fixed so it now can be `npm install`ed, so use it as a dev dependency.

pack: place archive in parent directory

e7afc8e

Current build directory may be temporary, get git cleaned, and the pack output might itself get packed. Avoid this mess by putting the pack file one directory up.

doc: add README.md and update --help output

0c93daa

bajtos reviewed May 27, 2014
View reviewed changes

sam-github added 6 commits May 27, 2014 21:09

onto: commit source tree onto a deploy branch

0530f3e

bundle: warn on problematic or incomplete config

a0ab12c

A missing .npmignore when .gitignore is present, or both bundleDependencies and bundledDependencies in package.json are problematic.

commit: commit build products into git

5ee4342

git: remove unused $0 argument to git.onto()

1e06a37

bundle,pack: log the actions taken

31ca4cb

sam-github added 3 commits May 27, 2014 21:13

fixup! TODO (will rebase away before when complete)

0315419

readme: update with latest usage

5a1928e

log: ensure each action logs on complete

8209351

sam-github closed this May 29, 2014

sam-github deleted the feature/build-a-package branch May 29, 2014 21:03

Uh oh!

Conversation

sam-github commented May 23, 2014

Uh oh!

sam-github commented May 27, 2014

Uh oh!

bajtos May 27, 2014

Choose a reason for hiding this comment

Uh oh!

sam-github May 27, 2014

Choose a reason for hiding this comment

Uh oh!

bajtos commented May 27, 2014

Uh oh!

bajtos commented May 27, 2014

Uh oh!

sam-github commented May 27, 2014

Uh oh!

rmg commented May 27, 2014

Uh oh!

ritch commented May 27, 2014

Uh oh!

sam-github commented May 27, 2014

Uh oh!

sam-github commented May 27, 2014

Uh oh!

sam-github commented May 28, 2014

Uh oh!

bajtos commented May 28, 2014

Uh oh!

sam-github commented May 29, 2014

Uh oh!

rmg commented May 29, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants