componentjs / component

frontend package manager and build tool for modular web applications
https://github.com/componentjs/guide
MIT License
4.55k stars 305 forks source link

deps set to * fail to fetch when no public tag is available #602

Closed gvilarino closed 10 years ago

gvilarino commented 10 years ago

Using 1.0.0-rc5.

Dependencies specified with * can't be found since the tags array from github is empty.

Try with this: component/aurora for example. Won't fetch 0.0.1 which is the master version, because there are no tags available.

The workaroun is to use master instead of * for these repos; the downside is that there are a LOT of these being called under official component repos, and I've had to fork out every dependency in my tree to fix this.

Maybe there's an easier way? I can't seem to find any documentation/issue about this, sorry.

sachalifs commented 10 years ago

:+1:

micky2be commented 10 years ago

-1

Seems normal to me. If there is no release/tag available why should it take master? * should mean the last release available, and not master.

dominicbarnes commented 10 years ago

-1

I agree with @micky2be, but I would also point out that master is not always the root branch. I've encountered a handful that use something else. (gh-pages for one, as weird as that is)

I don't want to add magic, but I also think this edge-case should be documented.

gvilarino commented 10 years ago

@micky2be @dominicbarnes I'm saying this because it's how 0.19.* used to work, and not supporting it breaks compatibility with components that haven't been updated since v1 is out. Even those under component github organisation.

My suggestion would be for * version number to work just as it is now, with the sole exception of fetching master when there are no tags at all in the repo. In a repo with no tags, master is the latest release available.

For instace, the repo I mentioned component/aurora is used by some other component in my dependency hierarchy. The only way I can think of to keep these working when migrating to v1 is to support * as fetch master when no tag is available. Builder could maybe issue a warning and tell you to be careful, but that shouldn't be a build-breaking issue.

trevorgerhardt commented 10 years ago

This is an example of a smart default, I'm all for this.

timaschew commented 10 years ago

to use * is usually a bad idea, unless you're using something like a npm shrinkwrap file, because you can't ensure to get things reproducible.

so if this breaks for some components, it's a signal to pin the * dependencies. you shouldn't fork the repos, just make a PR and if they won't merge it, then you should maybe use another component.

the downside is that there are a LOT of these being called under official component repos

we will fix that (add tags) for all our components within the component orga.

trevorgerhardt commented 10 years ago

@timaschew, I agree using * is generally bad idea, but it's ubiquitous. Using master by default if no tags are present creates less work and isn't an unexpected result. Isn't forcing tags equivalent to forcing a "publish" step like npm?

timaschew commented 10 years ago

but it's ubiquitous

That is not a reason to support these bad things ^^

Using master by default if no tags are present creates less work and isn't an unexpected result.

in which context? you mean when upgrading component from 0.19 to 1.0?

But that I think it's not really expectably when you are on the master using * and then someone push a commit which breaks your app. Any branch, if master or something else, representing your current working draft which is not guaranteed to work.

Isn't forcing tags equivalent to forcing a "publish" step like npm?

Yes, it is :)

gvilarino commented 10 years ago

@timaschew I agree with spirit behind the more restrictive approach to the use of *. I just believe it's not completely acceptable that 1.0.0 ships as is while official /component repos fail to build because no one tagged them.

But that I think it's not really expectably when you are on the master using * and then someone push a commit which breaks your app.

That would happen as well if someone publishes a new version without thorough testing. It's not a matter of tag/no tags, it's a matter of being mindful about what code is pushed/tagged. And let's keep in mind that what I suggest is only for those repos with the very rare -though real- case of not having public tags.

Any branch, if master or something else, representing your current working draft which is not guaranteed to work.

I believe nothing is actually guaranteed to work on the open source community. All softare is provided 'AS IS', as can be read in every other license.

Wrapping it up:

  1. In the ideal world * should fail when no tags are present.
  2. There are official /component repos that work, with no tags.

That being said, maybe supporting the very specific case of master when no tags are present is something that would make stuff work without the need of forking 6-7 repos in the hierarchy just to work around a tag that someone forgot to create, especially under /component.

This is all meant to improve adoption of component 1. The more rocks on the road someone meets when migrating is one more reason to drop component.

trevorgerhardt commented 10 years ago

This is all meant to improve adoption of component 1. The more rocks on the road someone meets when migrating is one more reason to drop component.

At this point, this is enough of a reason for me to disregard any downside of supporting a * to master fallback.

timaschew commented 10 years ago

That would happen as well if someone publishes a new version without thorough testing. It's not a matter of tag/no tags, it's a matter of being mindful about what code is pushed/tagged.

The frequency matters, in a active project you commit and push more often than release a new tag.

okay so let's do the fallback and make a console.warning with a deprecated note. and tell the people to pin it or change it to master. the fallback will be removed in component 1.1.0

gvilarino commented 10 years ago

That sounds awesome!

Thanks for the support :+1:

trevorgerhardt commented 10 years ago

So implementation for this would be in resolver.js correct? Any volunteers?

timaschew commented 10 years ago

yep :) I found and changed it already, need to do some test, then I'll push it

should be this function: https://github.com/componentjs/resolver.js/blob/master/lib/dependencies.js#L126-L150

timaschew commented 10 years ago

hmm I can't reproduce this, that's strange because I run into this issue as well some month ago

timaschew commented 10 years ago

@gvilarino master is used as fallback, here is the line: https://github.com/componentjs/resolver.js/blob/3b4a7a04663e7b99d4a5ec99a414fec6b04a0384/lib/semver.js#L141

So I close this issue unless you give me a script to reproduce the behaviour you described.

gvilarino commented 10 years ago

Hey thanks @timaschew ! Close away if you will; I'll try to provide a script that repros on rc5

timaschew commented 10 years ago

isn't it just component install component/aurora? This works for me, that's the reason why I'm asking to close, because the issues seems to be gone? Maybe github now returns not an empty array for tags anymore, because I don't see that the implementation of component has changed in the meanwhile.

So the issue should be open if we can reproduce what you have reported in your first post.

gvilarino commented 10 years ago

I understand and that makes a lot of sense. I'll try to make a repro situation later today and share it here

gvilarino commented 10 years ago

Well, that's pretty weird. It should fail as it did before, with a simple empty project and the following in their component.json

{
  "name": "foo",
  "version": "0.0.1",
  "description": "foo",
  "dependencies": {
    "component/aurora": "*"
  }
}

But it doesn't. Maybe the github API is actually working differently. I say just close it.