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

Multi-build: package filename length may exceed limit #170

Closed
wants to merge 1,543 commits into from
Closed

Multi-build: package filename length may exceed limit #170

wants to merge 1,543 commits into from

Conversation

asavoy
Copy link
Contributor

@asavoy asavoy commented Oct 16, 2013

When using steal to multi-build JavascriptMVC 3.3 apps into nice shared packages, it's possible to exceed the file-system length limit for filenames, which of course breaks the multi-build.

It occurs when there is a shared package comprised of enough long-named app names. This is because the package name is calculated by joining the app names together. This is the same issue as described in #169.

We've solved this for our own project by calculating an (8-character) MD5 hash of the package name, and using that instead. It also means some messy logic for dealing with potential filename collisions could be removed.

We've updated the multi-build test, so that ./js steal/build/test/run.js passes.

@matthewp
Copy link
Member

Very interesting! Will take a look at this as soon as I can.

@justinbmeyer
Copy link
Contributor

That's awesome, but I sorta like using the names if possible. It makes it easier to understand which packages belong to what modules.

Maybe a hybrid would work with this technique being used once file names got too long.

How long are your filenames? IE supports 1024 character urls.

@asavoy
Copy link
Contributor Author

asavoy commented Oct 17, 2013

@justinbmeyer

Actually we're reaching OS X's limit of 255 characters for the filename (including extension), at which point it will not write the file during build. (Indeed IE has a 1024 limit on URLs, but thankfully we're not at that point yet!)

While I agree it's nice to have the package names be meaningful, I argue it has little value in practice:

  • When the filename isn't short (say longer than 30 characters), it's too unwieldy to read and is often truncated in file listings, etc. anyway.
  • Unless debugging the multi-build itself, we've never needed to lookup the contents of production packages. (And we really are big users of the multi-build!)
  • If one really needs to know what's in a package, the first line in the file has a steal.has(...) which lists all the apps inside. And the multi-build process displays this information during build - if you really want to find where a module went, searching that output has been the best method for me, because it covers all packages/apps.

Regarding a hybrid approach, did you have something like this in mind?

  1. Generate the package name (as per original code)
  2. Generate the hash of the package name (as per this PR)
  3. The resulting name is a combination of the two, something like: packageName = friendlyName.slice(0, 16) + "-" + hash.slice(0, 8)

I could redo the PR to do that, if it will be accepted! :)

@asavoy
Copy link
Contributor Author

asavoy commented Dec 20, 2013

Kindly requesting feedback on this pull request! :)

The packaging process, prior to this pull request, will break once a certain number of modules is reached. Our production site has been using this fix for the last 2 months without issues. I took the simplest approach by using a hash for the filename, but I'm willing to tweak it based on any feedback.

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.