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

Publish without version prompt #3011

Closed
binarykitchen opened this issue Mar 29, 2017 · 25 comments · Fixed by #3646
Closed

Publish without version prompt #3011

binarykitchen opened this issue Mar 29, 2017 · 25 comments · Fixed by #3646
Assignees
Labels

Comments

@binarykitchen
Copy link
Contributor

Do you want to request a feature or report a bug?
minor feature

What is the current behavior?
yarn publish is prompting for a new version number

What is the expected behavior?
For a release and deployment bash script, I bump the new version internally and would like to pass on the new version to yarn with an argument like yarn publish --version new.version.number

And better, yarn publish auto, would parse the new version number in package.json. Sometimes I bump it there too.

Please mention your node.js, yarn and operating system version.
Think that's irrelevant?

@arcanis
Copy link
Member

arcanis commented Mar 29, 2017

You should be able to run yarn publish --new-version x.y.z to fix the version from the command line. Unfortunately, it seems impossible to just skip the version bump at all. Maybe the current behaviour should be moved inside a -i,--interactive flag ... wdyt, @bestander?

@bestander
Copy link
Member

Yeah, we should allow publishing without bump.
Maybe an argument --no-version-bump would be the easiest way to do it without breaking current API.
Send a PR!

@binarykitchen
Copy link
Contributor Author

but i am confused, code says there is already an argument for version, see
https://github.com/yarnpkg/yarn/blob/master/src/cli/commands/version.js#L70

@bestander
Copy link
Member

That one is yarn version command.

@binarykitchen
Copy link
Contributor Author

@bestander well, the code for the yarn publish command is using it, see https://github.com/yarnpkg/yarn/blob/master/src/cli/commands/publish.js#L138

@bestander
Copy link
Member

bestander commented Apr 6, 2017

So what happens when you pass version via --version?

@binarykitchen
Copy link
Contributor Author

ah, i think we can do yarn publish --new-version x.y.z ... will try tonight

@binarykitchen
Copy link
Contributor Author

binarykitchen commented Apr 8, 2017

hmm, tried that and am hitting the next stumbling block. with --new-version i see this error

yarn publish v0.22.0
info Current version: 1.25.7
error New version is the same as the current version.
info Visit https://yarnpkg.com/en/docs/cli/publish for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

let me explain my use case. i am running this bash script to deploy new releases:
https://github.com/binarykitchen/videomail-client/blob/develop/bin/release.sh

there you can see that i am tagging new releases with git flow, doing stuff on gulp, bumping the version, doing a final commit before pushing and publishing that to npm. in other words, the version bump already has been done and i don't want yarn to bump it.

honestly i think yarn shouldn't do this local check and not print the New version is the same as the current version. error. leave it to npm to respond with an error when the version already exist.

@binarykitchen
Copy link
Contributor Author

@bestander @arcanis if you think that's a separate issue, happy to raise another one

@bestander
Copy link
Member

bestander commented Apr 8, 2017 via email

@binarykitchen
Copy link
Contributor Author

here's the PR @bestander #3103

@bestander
Copy link
Member

fixed

@binarykitchen
Copy link
Contributor Author

@bestander is this still not out?

@bestander
Copy link
Member

It is in branch 0.24
screen shot 2017-05-11 at 10 59 55 am

We just keep it as RC till tomorrow but you can install it anyway.

@binarykitchen
Copy link
Contributor Author

@bestander now gave this a try and something does not look right. it gets stuck when publishing.

~/c/videomail-client ❯❯❯ yarn publish --new-version 1.27.6 --verbose                           ⏎ release/1.27.6 ✭
yarn publish v0.24.4
verbose 0.188 Checking for configuration file "/home/michael-heuberger/code/videomail-client/.npmrc".
verbose 0.188 Checking for configuration file "/home/michael-heuberger/.npmrc".
verbose 0.189 Found configuration file "/home/michael-heuberger/.npmrc".
verbose 0.191 Checking for configuration file "/home/michael-heuberger/.nvm/versions/node/v7.7.2/.npmrc".
verbose 0.191 Checking for configuration file "/home/michael-heuberger/code/videomail-client/.npmrc".
verbose 0.191 Checking for configuration file "/home/michael-heuberger/code/.npmrc".
verbose 0.191 Checking for configuration file "/home/michael-heuberger/.npmrc".
verbose 0.191 Found configuration file "/home/michael-heuberger/.npmrc".
verbose 0.192 Checking for configuration file "/home/.npmrc".
verbose 0.194 Checking for configuration file "/home/michael-heuberger/code/videomail-client/.yarnrc".
verbose 0.195 Checking for configuration file "/home/michael-heuberger/.yarnrc".
verbose 0.195 Found configuration file "/home/michael-heuberger/.yarnrc".
verbose 0.195 Checking for configuration file "/home/michael-heuberger/.nvm/versions/node/v7.7.2/.yarnrc".
verbose 0.195 Checking for configuration file "/home/michael-heuberger/code/videomail-client/.yarnrc".
verbose 0.195 Checking for configuration file "/home/michael-heuberger/code/.yarnrc".
verbose 0.196 Checking for configuration file "/home/michael-heuberger/.yarnrc".
verbose 0.196 Found configuration file "/home/michael-heuberger/.yarnrc".
verbose 0.196 Checking for configuration file "/home/.yarnrc".
verbose 0.198 current time: 2017-05-13T01:02:34.626Z
[1/4] Bumping version...
info Current version: 1.27.6
[2/4] Logging in...
[3/4] Publishing... <--- here it stucks and does nothing for minutes, not even a timeout

@binarykitchen
Copy link
Contributor Author

hello??

@binarykitchen
Copy link
Contributor Author

@bestander cc

@bestander
Copy link
Member

Thanks for reporting, @binarykitchen.
If it gets stuck then probably it tries to prompt for authentication.
Please open a new issue

@binarykitchen
Copy link
Contributor Author

@bestander why not reopen it? it's the same original issue - happy to examine and to submit another PR here

@bestander bestander reopened this May 29, 2017
@bestander
Copy link
Member

Thanks @binarykitchen, looking forward to get a fix

@binarykitchen
Copy link
Contributor Author

binarykitchen commented May 30, 2017

hmmm, i figured out that

stream.pipe(new ConcatStream(resolve))
  .on('error', reject);

in publish.js is the problem. when I added a data listener while debugging, publishing suddenly worked.

like that

stream.pipe(new ConcatStream(resolve))
  .on('data', console.log)  
  .on('error', reject);

probably the stream needs a data sink to make this actually work?

@bestander have an idea what that is?

@bestander
Copy link
Member

bestander commented Jun 15, 2017

@binarykitchen, thanks for reporting more.
There may be lots of reasons for streams to get stuck, e.g. https://github.com/ludios/why-are-my-node-streams-hanging.
Maybe adding a data handler affects scheduling a bit and your publish does not get stuck anymore but it seems like we might be handling streams piping wrong here.

Do you have a repo where this issue can be reproduced 100%?

@binarykitchen
Copy link
Contributor Author

yeah, good idea with data handler but i am afraid, i have switched back to npm5 and do not have the time right now :(

@bestander
Copy link
Member

@BYK, you said you have an idea where the bug is coming from.

@bestander
Copy link
Member

@binarykitchen, thanks for reporting anyway, hope we can win you back soon. :)

BYK pushed a commit that referenced this issue Jun 16, 2017
**Summary**

Potentially fixes #3011 (specifically https://git.io/vHAzW)

Turns out our `ConcatStream` implementation was only used to read
from `tar-fs` when creating a package to upload and put that stream
into a memory buffer. Since `ConcatStream` was implemented as a
`stream.Transform` and nothing was reading back from it, it had the
potential to just hang there until something reads from it. This
patch replaces that with [a small script][1].

[1]: http://www.geekpeak.de/images/produkte/i22/22-go-away-or-i-will-replace-you-de.jpg

**Test plan**

Removed existing tests for the old module. Rely on other existing
tests for the replacement code.
@BYK BYK closed this as completed in #3646 Jun 16, 2017
BYK added a commit that referenced this issue Jun 16, 2017
…am (#3646)

**Summary**

Potentially fixes #3011 (specifically https://git.io/vHAzW).

Turns out our `ConcatStream` implementation was only used to read
from `tar-fs` when creating a package to upload and put that stream
into a memory buffer. Since `ConcatStream` was implemented as a
`stream.Transform` and nothing was reading back from it, it had the
potential to just hang there until something reads from it. This
patch replaces that with [a small script][1].

[1]: http://www.geekpeak.de/images/produkte/i22/22-go-away-or-i-will-replace-you-de.jpg

**Test plan**

Removed existing tests for the old module. Rely on other existing
tests for the replacement code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants