Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fatpacked, standalone recs command #47

Merged
merged 18 commits into from
Oct 16, 2014
Merged

Conversation

tsibley
Copy link
Collaborator

@tsibley tsibley commented Oct 13, 2014

A pure-Perl, single-file "recs" command that lets you run operations like recs xform ... which should work on Perl 5.8.1 and higher without any additional modules other than those in core Perl.

To try it out:

curl https://raw.githubusercontent.com/tsibley/RecordStream/fatpackable/recs > recs
chmod +x recs
./recs --help

In the process I moved to Dist::Milla (a nice wrapper around Dist::Zilla) for distribution management to make the build and release process simpler, as well as to take advantage of the better dependency tracking provided by a cpanfile + Dist::Zilla.

The commit messages are worth reading for more details. I wanted to get this pushed up and being looked at first, but I plan on revamping some of the POD documentation too.

These look to be relics missed by the cleanup in 4fc9555 ("oops").

"use lib" with no paths does effectively nothing to @inc.  The code
which follows doesn't call lib->import() manually or similar, so these
lines don't serve any purpose.
@tsibley
Copy link
Collaborator Author

tsibley commented Oct 13, 2014

Hmm, may need another tweak to work on 5.8.x. Just ran into an issue! :)

@tsibley
Copy link
Collaborator Author

tsibley commented Oct 13, 2014

I'll fix that soon, but nothing will change dramatically.

@tsibley
Copy link
Collaborator Author

tsibley commented Oct 14, 2014

JSON::PP issue fixed on Perl < 5.14. In using the fatpacked version for my daily use for part of the day today, I ran across an issue with aggregators because BaseRegistry scans @INC.

This helps clarify the dependency chain and opens up structured optional
features for users.
Dist::Milla is a simply a nice out-of-the-box configuration of
Dist::Zilla.  Several regular Dist::Zilla plugins are used as well.

A Makefile.PL is kept checked in so that only authors making releases
need to use Dist::Milla.  Everyone else just installs and uses the
repository as usual.  The Makefile.PL (and other generated files) may be
updated at any time by running:

    milla build --no-tgz && milla clean
@benbernard
Copy link
Owner

This is great, I made a single file version of recs before using, I think, pp or something, but I think this is better, just have a few comments...

@benbernard
Copy link
Owner

I think we should put the procedure for updating the distrobutions somewhere obvious (maybe the devel directory?) I'm talking about the milla build --no-tgz && milla clean stuff

@benbernard
Copy link
Owner

(hard to make line comments because github is barfing on the # of files changed)

How do the 'recommends' requirements show up if you just do a install App::RecordStream? I'd like those installs to get an XS version of JSON parsing at least (don't care as much about CSV)

@benbernard
Copy link
Owner

Just a question here, should we have tests for the flatpack version? You meantioned some stuff, but it would be nice to have a script or something to run that for us

@benbernard
Copy link
Owner

on update-fatlib script, we should probably use 2 space indenting like the rest of the code (is supposed to use)

@benbernard
Copy link
Owner

thats about it! thanks for the awesome work!

tsibley and others added 6 commits October 14, 2014 16:55
Note that this preserves the bug of two hash keys for README.pod,
leaving only recs(3) installed and not RecordStream(3).  Since all the
documentation refers to RecordStream(3), I will be fixing this in the
future during a documentation update.

I fully intend for this to get less ugly in the near future by changing
how the docs are built (and slimming down the dist.ini again).
Some of the operation docs incorporate system-specific values (such as
fromps) into their documentation.  This simply restores the previous
behavior before the Dist::Zilla conversion.
Don't require XS versions of JSON and Text::CSV.  The pure-Perl versions
are slower, but work.  Where the XS versions are available, they'll be
automatically used.

Switches to JSON::MaybeXS while we're at it, since it has a) better
dependency handling/detection and b) prefers Cpanel::JSON::XS over
JSON::XS.  The former is more reliable, community-driven, and more
developer-friendly than the latter.  JSON::MaybeXS will use JSON::XS if
it's the only XS option available.  Failing any XS available, it falls
back to JSON::PP of course.

Term::ReadKey is nice, if we have it, but otherwise relying on $COLUMNS
from the environment and defaulting to 80 is very reasonable.

Making these three XS deps optional means it's possible to fatpack recs.
…egators

It's better than the previous ad-hoc method in several ways (e.g. more
resilient to unexpected files), but most importantly version 5.1 and
newer know how to play nicely with App::FatPacker's @inc entry.  This
ensures that core packages are located even when fatpacked.

Module::Pluggable itself comes with no non-core deps, which is great.
(Though it used to be core itself, it no longer is and, besides, the
version we need was never in core.)
…a git

This allows for git-style invocation of subcommands such as:

    recs fromcsv --header ...

More importantly, having a single command provides an obvious
point-of-entry for fatpacking the builtin operations into a single
bundle.

Similar to git, all that's necessary to add a subcommand is having an
executable in the PATH beginning with "recs-".  Operations are
preferentially loaded from the Perl lib paths, which includes the
builtins provided by the core distribution.  This saves a re-exec of
perl when it isn't necessary and will also run core operations when the
whole dist is fatpacked into this script (and hence there aren't
seperate recs-* scripts).
@tsibley
Copy link
Collaborator Author

tsibley commented Oct 15, 2014

Yep, I saw the existing single-file stuff that used PAR, but doing it as pure-Perl is much nicer.

I pushed a rebased version of the branch that cleans up some of the initial missteps, as well as a fix for the BaseRegistry issue.

Re: procedure for updating the distribution — it's now in devel/README. As noted there, if you install Dist::Zilla::App::Command::update, it's as easy as milla update.

Re: recommends deps when installing — I will check on this and report back. It will vary between CPAN clients, so I'll check a few.

Re: tests — Yep, I've been planning to and will push some up soon. :)

Re: two-space indents — Good point. It started based on a similar tool from cpanminus, so I kept the four-space indent, but it's different enough now that I think changing to two-space for this project makes more sense. I'll do that.

We want to keep track of changes to both of them.  The local/ directory
contains arch-specific packlists and paths which must match the current
machine when fatpack runs.  For this reason, we don't keep local/
checked in.

Note that local/, fatlib/, and the fatpacked recs script are all
excluded from the CPAN dists so that they don't increase the dist size
and aren't indexed by PAUSE by mistake.
Otherwise it won't be fatpacked when building on Perl 5.14 or higher,
which includes JSON::PP in core.  To support older Perl's, we explicitly
specify it.
This can help local debugging if something unexpected pops up.

For example, it revealed that Date::Manip uses %+ which is a Perl 5.10
feature (via Tie::Hash::NamedCaptures).  Luckily for us, the DM5 backend
doesn't require 5.10.

Modules which are known OK to skip are marked as such.
@tsibley
Copy link
Collaborator Author

tsibley commented Oct 15, 2014

How do the 'recommends' requirements show up if you just do a install App::RecordStream? I'd like those installs to get an XS version of JSON parsing at least (don't care as much about CSV)

Systems which can compile XS modules (and don't ask for pure-Perl only) will get Cpanel::JSON::XS if they don't already have it or JSON::XS installed. This is thanks to the dynamic dependencies of JSON::MaybeXS. Text::CSV doesn't work the same way and folks will have to install Text::CSV_XS themselves to get the speedup. (There is the option of creating Text::CSV::MaybeXS which is the equivalent of JSON::MaybeXS, and then depending on that for App::RecordStream. For my purposes, this is appealing, and I may do so.)

Regarding the optional features, both the traditional cpan (CPAN.pm) and cpanp (CPANPLUS) clients do not understand that portion of the metadata. This is the same behavior from the pre-cpanfile state where the optional features were just commented out in Makefile.PL for anyone browsing by. cpanm (App::cpanminus) will prompt for which features are wanted when run with --interactive and also accepts flags which enable specific/all features (--with-feature=..., --with-all-features). cpanm also has --with-recommends and --with-suggests options. It is by far the client with the most comprehensive detailed prereqs support.

@tsibley
Copy link
Collaborator Author

tsibley commented Oct 15, 2014

I think that covers everything! I'm using the fatpacked version of recs on a number of machines for daily use now to try and catch any more issues.

@tsibley
Copy link
Collaborator Author

tsibley commented Oct 15, 2014

I just learned that cpan (CPAN.pm) also has support for top-level recommends/suggests via configuration options.

@csmarshall
Copy link

+1 a carry along recs tools has been on my wish list for so long!

Sent from my iPhone

On Oct 14, 2014, at 16:50, Ben Bernard notifications@github.com wrote:

thats about it! thanks for the awesome work!


Reply to this email directly or view it on GitHub.

benbernard added a commit that referenced this pull request Oct 16, 2014
fatpacked, standalone `recs` command
@benbernard benbernard merged commit e1796e3 into benbernard:master Oct 16, 2014
@benbernard
Copy link
Owner

Merged! We should probably get that info you discovered about installation stuff in a README or something, but great work :).

@tsibley
Copy link
Collaborator Author

tsibley commented Oct 16, 2014

Thanks! :)

Next on my plan is updating the overview and installation docs to talk about the fatpacked version and various ways of installing from CPAN + working with the git repo. I suspect that'll come as another branch soonish.

In the meantime, I'm going to make a trial release (unindexed by PAUSE) to CPAN to see what, if anything, shakes out from CPAN Testers.

@tsibley tsibley deleted the fatpackable branch October 16, 2014 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants