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

Enter nested job scope per package architecture when downloading #4163

Conversation

@OleksandrChekhovskyi
Copy link

@OleksandrChekhovskyi OleksandrChekhovskyi commented Apr 9, 2015

This fixes a crash when two architectures need to be downloaded
for the same package. It happens when building for non-host architecture
if packages have not been cached yet.

Crash happened because progress object got reused for another download request,
and progress objects are not supposed to be reused.

How to reproduce:
Build any meteor project for os.linux.x86_64 when ~/.meteor is empty

CLA has been signed by the company (Hansoft) and sent via e-mail

This fixes a crash when two architectures need to be downloaded
for the same package. It happens when building for non-host architecture
if packages have not been cached yet.

Crash happened because progress object got reused for another download request,
and progress objects are not supposed to be reused.
@apollo-cla
Copy link

@apollo-cla apollo-cla commented Apr 9, 2015

@OleksandrChekhovskyi: Before we can merge your pull request, you'll need to sign the Meteor Contributor Agreement: https://contribute.meteor.com/

@glasser
Copy link
Member

@glasser glasser commented Apr 14, 2015

I wasn't able to reproduce this. Can you provide a more specific reproduction, with specific packages that are used (and what do you mean by ~/.meteor being empty, since that implies you don't have a tool to run either...)

@OleksandrChekhovskyi
Copy link
Author

@OleksandrChekhovskyi OleksandrChekhovskyi commented Apr 15, 2015

Apparently it does not happen with a newly created/empty app. However, adding accounts-password will make the bug happen.

Detailed reproduction steps (when running meteor from git checkout):

[meteor checkout]/meteor create myapp
cd myapp
[meteor checkout]/meteor add accounts-password
rm -rf [meteor checkout]/.meteor
[meteor checkout]/meteor build .. --architecture "os.linux.x86_64"

You will get this as output:

/Users/oleksandrchekhovskyi/extern/meteor/tools/progress.js:213
      throw new Error("Progress transition from done => !done");
            ^
Error: Progress transition from done => !done
    at [object Object]._.extend._updateTotalState (/Users/oleksandrchekhovskyi/extern/meteor/tools/progress.js:213:13)
    at [object Object]._.extend.reportProgress (/Users/oleksandrchekhovskyi/extern/meteor/tools/progress.js:152:10)
    at Request.<anonymous> (/Users/oleksandrchekhovskyi/extern/meteor/tools/http-helpers.js:268:20)
    at Request.emit (events.js:95:17)
    at emitProgress (/Users/oleksandrchekhovskyi/extern/meteor/tools/http-helpers.js:292:15)
    at Request.<anonymous> (/Users/oleksandrchekhovskyi/extern/meteor/tools/http-helpers.js:305:9)
    at Request.emit (events.js:95:17)
    at Request.onRequestResponse (/Users/oleksandrchekhovskyi/extern/meteor/dev_bundle/lib/node_modules/request/request.js:1247:10)
    at ClientRequest.emit (events.js:95:17)
    at HTTPParser.parserOnIncomingClient [as onIncoming] (http.js:1693:21)
    at HTTPParser.parserOnHeadersComplete [as onHeadersComplete] (http.js:122:23)
    at CleartextStream.socketOnData [as ondata] (http.js:1588:20)
    at CleartextStream.read [as _read] (tls.js:514:12)
    at CleartextStream.Readable.read (_stream_readable.js:341:10)
    at EncryptedStream.write [as _write] (tls.js:369:25)
    at doWrite (_stream_writable.js:226:10)
    at writeOrBuffer (_stream_writable.js:216:5)
    at EncryptedStream.Writable.write (_stream_writable.js:183:11)
    at write (_stream_readable.js:602:24)
    at flow (_stream_readable.js:611:7)
    at Socket.pipeOnReadable (_stream_readable.js:643:5)
    at Socket.emit (events.js:92:17)
    at emitReadable_ (_stream_readable.js:427:10)
    at emitReadable (_stream_readable.js:423:5)
    at readableAddChunk (_stream_readable.js:166:9)
    at Socket.Readable.push (_stream_readable.js:128:10)
    at TCP.onread (net.js:529:21)

Note that "meteor add" will cache the package already for the current architecture, so removing the package cache is necessary to fully simulate required conditions.

This situation is unlikely to happen on a developer's machine, as running the app will cache packages for the current architecture. In our case this crash happened on a build bot which did not have anything cached yet.

@glasser glasser closed this in cc21d89 Apr 22, 2015
@glasser
Copy link
Member

@glasser glasser commented Apr 22, 2015

Thanks for the fix and the good reproduction!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.