This mostly fixes tests:
- removes the 'restarted' check from some tests. We don't need it in those cases
(printing the other banner is enough). We can no longer rely on that executing
after the code in the package (in fact it seems to execute before, and then
get overwritten), and the test still tests what it is intended to (that the new
package code executes).
- minor fixes to essentially syntax errors -- the skeleton now uses double quotes
instead of single quotes, so a regex failed to work, for example. We changed a
version number in one part of the test, but not another.
- fixes selftest.js, sort of, to actually print out what test we are testing. This
is an unfortunate interaction of Console.js changes in 1.0.2 and a progress bar
(that came later). The progress bar erases the message telling you what test is
running when you use a standard terminal. That's awkward, fixed.
Summary:
When running 'meteor show <packageName>' show
Package: <packageName>@<defaultVersion>
(instead of "Package: <packageName>" )
The default version is the version number of the version record
that acts as the source for exports, implies, long description, etc.
It is the local record (in which case, we will show "@local" to be
more clear); if there is no local record, it is the highest semver mainline
record (ie: not a pre-release) and if *that* doesn't exist, it is just
the highest semver record that we have.
Test Plan: self-test show --slow
Reviewers: glasser
Differential Revision: https://phabricator.meteor.io/D8
- Package: <name>@<version> rather than
Package: <name>
Version: <version>
in the version output
- Remove the summary line from version output.
Move 'Published by' to the bottom as a separate line.
- Move git up.
The impetus behind these changes is to reduce the size of the header on 'meteor show'.
We thought that the long paragraph of "Foo: Bar" type things was too overwhelming.
Some more changes:
- Clean up an extra line that comes up when printing the description sometimes.
- Add 'This package version is built locally from source' to the message about versions
available on the server.
- For releases, process the "non-recommended versions have been hidden message" for the single-hidden-version
case, in the same way that we do for packages.
In 'meteor show', display the list of packages implied by a package/version.
Implied exports are part of the package's exports, especially for an umbrealla
package like 'meteor-platform' or 'cfs:standard-packages'. However, we can't
tell you the exact exports (ex: "Mongo") without running the constraint solver
(because we don't know what version of the implied package you will end up with).
Showing implies also makes umbrella packages like 'meteor-platform' and
'cfs:standard-packages' more obvious -- the user can tell what is going on with the
package much better.
This commit is based on the following design document:
https://mdg.hackpad.com/Creating-and-Updating-Docs-0ZyyDcSZDxp,
and some other stuff from here: https://mdg.hackpad.com/Meteor-Long-Description-wGZ1vIOwVlF
and was code reviewed here: https://github.com/meteor/meteor/pull/3375
It does the following:
- Allow the user to specify package documentation in Package.Describe.
We will take the README.md file by default, to make the transition easier.
Users can specify ‘documentation: null’ to not submit a README.md
- From that documentation, extract the section between the first and second header
to use as the long form description for the package.
- Upload the documentation to the server at publish-time. Allow metadata changes with ‘publish —update’.
- Change the default package skeleton to include the README.md file.
Also, changes the skeleton to have fewer useless placeholders in Package.describe values.
- Fix a minor bug where Git did not show up when running ‘meteor show’ on local packages.
A note on ‘documentation: null’ and blank documentation — we don’t let maintainers upload
blank README.md files, because we want to encourage people to fill them out. (Instead,
we allow a ‘documentation: null’ as an override) This is a UX issue! It is not a technical thing.
There is more discussion and code review in: https://github.com/meteor/meteor/pull/3375
Contains:
- method to aggregate exports for a package in packageSource (exports are per-architecture).
- get this data from packageSource in PackageQuery for ‘meteor show’. Don’t store it in the
local catalog — while it is not a particularly expensive operation, it is still more expensive
than a simple lookup. We really do care about minimizing any sort of computation when we
are initializing packages, since we want the tool to be fast.
- display the data in ‘meteor show’. It makes sense to line wrap this with the ‘Exports:’ label as a
bulletPoint (just look at the test to see an example where this improves user experience). Since we
are doing that, we might as well use that bulletPoint functionality on the other labels as well.
- There is also a test. Run ‘meteor self-test show’ to test, or run ‘meteor show’ on a local package
with exports.
The Troposphere counterpoint to this is: meteor/troposphere#5
This is a thing that I wanted to try -- running 'meteor show' in a
package directory shows you that version's data.
- You might want to run 'meteor show' to get export or dependency
information on a local package, instead of looking through the
package.js file.
- Before publishing your package, or updating its metadata, you might
want to make really sure that its longform description looks good
in 'meteor show'. Hopefully it does! I would want to check.
Running 'meteor show <name>@local' from a package directory feels
slightly janky to me.
- Other commands in the publiction workflow read 'package.js' to figure
out your package name. It feels weird to type it out.
- Many package names don't correspond to the directory name. It is good
to help the user spend less time inspecting package.js files for
obvious information.
This has bothered me a lot during testing, which is not a normal workflow.
I might be somewhat biased here, in a way that normal users would not be.
There is a minor inefficiency around retrieving a local version record twice,
but I think that it is worth it for code simplicity/readability/etc.
Instead of the generic "Some versions of X have been hidden"
message when only showing some versions of a package, use a more
detailed message. For example:
- "Older versions of X have been hidden"
- "Older, pre-release and unmigrated versions of X have been hidden"
- "One older version of X has been hidden."
There is some hand-waving around the logic resolving what to do about,
for example, old pre-releases. Overall, we want to err on the side of
having a clear and obviously consistent user experience:
- any version less than the lowest shown version (ex: 1.0.0-rc.0 vs 1.0.0)
is an 'older' version. Sometimes, that version is also a pre-release. It
is possible that if we were NOT filtering out pre-releases, we would show it.
We still respond that it is ‘older’, because that seems more obviously consistent.
- we report any ‘pre-release’ or ‘unmigrated’ versions in the version interval
that we show. That is, if we are showing ‘1.0.0’ and ‘2.0.0’, and hiding
2.0.0-rc.0 and the un-migrated 1.1.0, we will mention it.
Of course, that interval does depend on what versions we choose to hide. It is
possible to imagine a situation where we don’t hide pre-releases, in which case,
‘1.0.0’ above might not make the cut, and neither would 1.1.0. Luckily, we either
show everything, or hide everything, so this is only theoretical.
The ‘show’ command has been completely rewritten. It has different output
and now does the following:
- Interacts with local package versions. Checks in the local package catalog, and
returns the local versions along with the server versions. When ‘meteor show’ is
run with a specific version request (‘meteor show foo@<version>’), default to
showing the local package version (but show a message that a server version is
available). Running ‘meteor show foo@local’ will always show the local version
(useful for version-less local packages).
- Simplify the interface. Instead of various ‘show-*’ flags, we only have one: show-all.
By default, we only show the top 5 official (non-prerelease) unmigrated versions of a
package (+ local version, if applicable). This can be overridden with ‘show-all’, and we
let the user know that more versions are available. For releases, ‘show-all’ will show
non-recommended releases.
- Display publication time for non-local package versions. This makes it easier to run
‘meteor show <name>’ and see if <name> is actively maintained. For local packages,
we display the root directory (useful for large apps or running with the
LOCAL_PACKAGE_DIRS variable, for example).
- For non-local package versions, show if the version is ‘installed’ (downloaded into the
warehouse). This involved minor changes to tropohouse.js. The idea is that this should
give a pretty good clue whether the version can be added offline.
- Show version dependencies. This should help the user understand, track down and
debug constraint solver failures.
- Do not show version architectures except in —ejson mode.
- Allow an ‘—ejson’ flag to get the output in EJSON format. That should make scripting
easier. (As a bonus, for release versions, the EJSON output acts as a nice template
for the release configuration file.)
The search command now does the following:
- Interacts with local package versions. Specifically, local versions override equivalent
server versions. Also, ‘search’ works on local packages (so, for example,
‘meteor search troposphere’ inside the package server app will give you the troposphere
package).
- Allows an ‘—ejson’ flag to get the outout in EJSON format.
Minor changes to some minor testing infrastructure:
- A new skeleton package, package-for-show. Its versions contain different
values for various metadata, so we can test that metadata comes from
the right version.
- In several places, replace the pattern of copying around
package.js files with using the replace function on a placeholder
string. (Mostly, as applied to package versions).
This is based on these hackpads: https://mdg.hackpad.com/Showing-Package-Metadata-HdGo3Lzx3hR
and https://mdg.hackpad.com/Meteor-Search-Output-1xxEzrAK9YU.
This will be useful when we want to be smart with windows file paths later
Also, all of the file calls are asynchronous with fibers now, which comes with
many benefits.
This is a combination of 23 commits. Original messages:
Wrap a large number of fs calls inside files.*
Convert a few more fs calls to files.*
More moving fs.* to files
Implement read/write streams and open/read/close
Get rid of fs from auth.js
Remove fs and unused imports from catalog-local and catalog-remote
Remove unused imports from catalog.js
Replace a whole lot of fs calls
Fix error
Migrate a lot more fs. calls to files.
Add a temporary symlink method
Convert old test to files.*
Use files.pathX instead of path.x everywhere
Replace path.x to files.pathX in tests
Small fixes to files.js and one rename
Make cleanup run in a fiber
Make wrapping functions take function name in case we need it
Add some timeouts and stuff to HCP tests
wrapFsFunc also makes a sync version of the function
Sometimes you just don't want to yield!
Make sure JsImage readFromDisk doesn't yield
Remove unused imports from npm test
Change order of test now that some things don't yield
Fix missing files import, and add a debug error printout
Previously, we would register the circular dependency error properly
with buildmessage, but then try to build the package with a circular
dependency anyway, leading to a crash.
Fixes#3280.
Fixes#2846. Fixes#2847. Fixes#2979.
Errors in the build process that could be fixed by refreshing the
catalog now cause the catalog to refresh, assuming we have not already
refreshed it recently and that we are not offline.
These commands now don't need to refresh at startup: remove, run, debug,
create, build, bundle, deploy, test-packages, rebuild, and self-test
It should be OK for create to throw SpringboardToLatestRelease even
without refreshing, since release.latestKnown is still something we know
about.
Includes the following changes to Console.js:
- Console.info, Console.warn, Console.debug and Console.error now automatically
line-wrap the output to 80 characters, or the width of the terminal screen (if
known). This is in line with our current style guide on how things should be wrapped!
- Sometimes, there are parts of text that we don't want to line-wrap. For example, if we are
telling the user to run 'meteor long-command --with --options' we don't want to
have a newline in the middle of that! Wrap those commands in Console.command, like
this:
Console.info("something and then run", Console.command(command), "and then");
This also makes them bold if chalk is on, as a nice bonus. So, if we ever turn
chalk back on, the bolding of commands will be more consistent.
- Sometimes, there is bulkier output that we don't want to format at all, including
line-wrapping: log snippets, stack traces, JSON output, etc. In that case, we can use
Console.rawInfo, Console.rawError, Console.rawWarn and Console.rawDebug. Don't use
Console.command inside the raw* functions! It won't be processed (at all).
- There are fancier things that we can do, other than just simply wrapping things.
We can indent:
" Start here and then when wrapping
continue over here".
We frequently do this for commands, for example. In the past, we did this manually --
but we can't do this for long messages that might get wrapped, and anyway, it is
good to codify this instead of counting spaces. Allows us to be better about consistency,
for example.
- We can also add a bulletPoint, which is a small notice in the beginning that looks like
this:
" => Start here and then when wrapping
continue below the bulletPoint".
Since it is a elss intuitive option, I have wrapped most of the time that we use a
bulletPoint into helper functions on the Console.js.
- Some common bulletpoints that we use are:
ASCII Checkboxes (Console.success)
ASCII X-s (Console.failWarn and Console.failInfo)
=> (Console.arrowError, Console.arrowWarn, Console.arrowInfo)
WARNING (Console.labelWarn)
The => are sometimes indented, so they take an optional indent argument, showing how
many spaces to indent by.
The wrapper interface would be less complicated, if there was a more unified conceit behind our
terminal messages. If there is one, it is not documented. My hope is that, in many cases,
moving these to Console will make it easier for someone with great product sense to
clean up our terminal messages. It will also make it easier to write such messages, since
it will be easier to follow an accepted standard.
In the codebase outside of Console:
- Went through and looked at our use of Console.error/info/etc, replacing with rawError/etc
whenever approporiate.
- Went through and modified most of 'stdout' and 'stderr' calls to use the new functions.
I made an exception for stuff that doesn't want a new line at the end, or otherwise does
weird things (ex: print user logs directly), on the basis that, at this juncture, it is
better to be safe than to be sorry.
- Long messages no longer need to break the code style guide by ignoring indentation rules.
Fixed that where approporiate.
- Fixed the tests! A number of our stock messages are actually longer than 80 chars.
- Personal favourite: The Android license agreement is now line wrapped! Much better experience.
- There is some more work to do on:
- longform help (currently comes with built-in linebreaks, would have to change the entire
mechanism for how that works)
- Buildmessage sometimes has headers that start with =>, but they are short. I didn't want to
pass wrapper options all the way to main.captureOrExit before merging the rest of this and
making sure that we like it. Since these messages are fairly short, I don't think that's
likely to be a serious problem.
I hope that this makes life easier for us in the future! No more counting chars, no more breaking
the style guide. Better experience for users with wider terminals (or even shorter terminals!).
Let's give this a try.
This reimplements functionality that had been removed as part of the
`isopack-cache` branch refactoring.
Information about package changes is encapsulated inside a
PackageMapDelta object on the ProjectContext. It is the responsibility
of the command that prepares the ProjectContext to choose to call
projectContext.packageMapDelta.displayOnConsole at the appropriate time
if it wishes to display changes.
Part of #3006.
(still outstanding: changes to package publication workflow)
A package marked debugOnly in the package source is not to be bundled in production.
Moreover, if a package/app depends on a debugOnly package, that entire tree should
not be bundled. (But we should take it into account when figuring out versions!)
Does the following:
- In the catalog, we have a function that takes in a set of versions and a set of original
constraints and traverses it, recursively, to build a subset of versions that we *should*
bundle, and the corresponding subset of versions that we shouldn't (because they are either
debugOnly themselves or pulled in by debugOnly packages). (We do this in the catalog because
it is an addon onto the results of the constraint solver, tied deeply into our representation
of data)
- In the packageLoader, we keep track packages & versions that we should bundle, and also,
packages that we should exclude. We do this in the package-loader because, essentially, that's the
object that we use to keep the results of the constraint-solver, and we already propagate it to all
functions that care about it. (Possibly we should subsequently rename it later).
- In the compiler, when we figure out buildTimeDependencies, we ask if we need to bundle debug
builds. If not, we filter them out (see above). Also, when we actually build together unibuilds,
we don't touch the ones that the packageloader tells us to exclude (which ensures that they don't make
it into the final product).
- In the project, we keep track of whether this project is building in debug mode. That's because the project
is where we keep the state of our curent project that we are building, and if we are ever in the state of
building multiple things, then that's the code that we would need to touch (see also that we make a similar
assumption when solving constraints).
- Adds the option to keepthe project debug-build-free and calls it in commands when approporiate.
When we publish things to the test packaging server, we use the versionsFrom
argument that is very difficult to set up right to actually work. Most of the time
we don't really set that up and just agree that those tests should fail. As such, I
am going to mark most of those tests as checkout-only for now (we still usually check
that we can publish manually anyway as part of poking at the release in QA, and there is
not a lot of reason that I can think of why publishing from release would be different than
pubishing from checkout. So, marking these as checkout-only until we can get a better
testing infrastructure for this in place.
Rewrote parts of the update server package data unit test to compare a pre-sync and a
post-sync catalog, rather than the output of package client's attempts to contact the server.
This is because in the new world, there is no accurate output, and instead, the function
modifies the catalog in place. Additionally, removed the old function that read from
data.json, since it was not used anymore, and cleaned up some comments and return values in
package-client. We no longer claim to return the contents of data.json, instead we return
an object that signals if we should reset the entire catalog, and/or if our connection to the
server flat-out failed. I am not sure that this is the best abstraction for this piece of code
(why does package-client modify the catalog, but not reset it, for example? Since resetting
has consequences, in the ideal world, the package-client would only have the logic to get data
from the server and it would be up to the catalog to figure out how to insert it into sql lite,
I think, maybe. Regardless, now is not the time to do that refactoring.)
The test is a little odd in the following ways. First, comparing every record ever published is
something that is already significantly harder than it used to be, and will get only harder from
there. (However, the test claims to check that no data has been erased, so we need to check it).
We check the vague existence of most records, on the theory that it is unlikely that
we only got a portion of the record in, rather than the entire thing. Second, it doesn't check the
actual contents on disk, because I wasn't sure about writing another interface to sqllite this
late in the game. There are some ways to get around this -- we could be sneaky and submit a non-blank
syncToken in some way (faking a previous sync), so we only get the packages touched since (time X).
Usually, that might violate some internal consistency, but we only care about the contents at this stage.
Second, we should probably write some method on the catalogs to compare themselves instead of making
and querying a copy.For now, though, I think that this is sufficiently expedient.
Also, the test publishes 5 packages. That's a lot of packages, so I marked it as slow.
