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

Caret by default #41

Closed
sdboyer opened this issue Jan 17, 2017 · 21 comments
Closed

Caret by default #41

sdboyer opened this issue Jan 17, 2017 · 21 comments

Comments

@sdboyer
Copy link
Member

sdboyer commented Jan 17, 2017

We need a way of simulating carat-by-default behavior. That is, if you call semver.NewConstraint("1.0.0"), it will interpret that as "^1.0.0". To get only and exactly that version, you'd have to call semver.NewConstraint("=1.0.0").

When I started looking at this, it had more far-reaching implications than I initially thought :(

@mattfarina
Copy link
Member

To restate, this is about switching the default, when no range is provided, from = to ^.

What is the motivation? Use case? I'm not familiar with the background and would like to come up to speed.

@sdboyer
Copy link
Member Author

sdboyer commented Jan 18, 2017

To restate, this is about switching the default, when no range is provided, from = to ^.

Yup.

What is the motivation? Use case? I'm not familiar with the background and would like to come up to speed.

The pm committee decided to do this a while back. The motivation is providing a nudge in the direction of being open within what should be a safe range. It's also taking a cue from cargo.

The string "0.1.12" is a semver version requirement. Since this string does not have any operators in it, it is interpreted the same way as if we had specified "^0.1.12", which is called a caret requirement.

@mattfarina mattfarina modified the milestone: 2.0.0 Jan 19, 2017
@mattfarina
Copy link
Member

Before I say go forth... I'm curious if this should be in the semver library or the application code that passes it in. I'd like to ponder this.

@technosophos any thoughts? I ask because helm uses semver.

@sdboyer
Copy link
Member Author

sdboyer commented Jan 19, 2017

I think I'd considered doing it that way in the past, but...well, I can't remember offhand what stopped me. I think there may end up being subtle dependencies that make it hard for gps to ensure everything's done correctly, at least.

I'll look into it again, though.

@mattfarina
Copy link
Member

@sdboyer I just want to take a moment to consider multiple angles. Don't beat yourself up over it.

@mattfarina
Copy link
Member

@sdboyer what do you think of providing the ability to set the default when no range is supplied? Then setting the default default to ^.

@sdboyer
Copy link
Member Author

sdboyer commented Jan 25, 2017

@mattfarina That might work. Could you possibly work up some quick pseudocode to demonstrate what you have in mind, to make sure I'm understanding your suggestion correctly?

@technosophos
Copy link
Member

Ugh.. please don't change the default to "If I specify an exact version, I mean not an exact version." That's sooooo not user friendly, and soooo backwards compatibility breaking.

@sdboyer
Copy link
Member Author

sdboyer commented Feb 1, 2017

That's the plan for how it's going to work in dep. The only question (in this issue) is how that integrates with what's here.

@technosophos
Copy link
Member

Yeah, Dep can do whatever. I have no preference about that. But changing Semver is a breaking change, and since this is a string format thing, it would require us (for example) to potentially make thousands of changes to existing Helm charts. it would also potentially break backward compatibility with Glide, which (as you know) has a pretty substantial user base at this point.

@sdboyer
Copy link
Member Author

sdboyer commented Feb 1, 2017

But changing Semver is a breaking change

Right - this would certainly only be for the 2.x line.

My concern there is that, if dep is to implement autoconversion from glide.yaml files - which it almost certainly will at dep init time, though it's not yet clear if it will during solving - then either semver 2.x will need to adaptively support either mode of operation, or...well, I'm not sure there's an or. And, as you note:

it would also potentially break backward compatibility with Glide

Adaptively supporting either mode is tricky, not at the string parsing layer (there could be a different entry point, or a flag on NewConstraint that allows the user to choose carat-by-default or not), but at string generation time - Constraint.String() won't know whether to render a carat-expressible range with or without the carat.

We could also overcome that with a special ToString(c Constraint, cbd bool) string wrapper that lets the user designate carat-by-default behavior (same idea as with the inputs), but then we're breaking Stringer simplicity.

It's ughs all around.

@technosophos
Copy link
Member

Sounds to me like the path of least resistance is for dep to continue to use x.y.z as implying an exact version, not an implied ^.

@sdboyer
Copy link
Member Author

sdboyer commented Feb 3, 2017

Oh, that would definitely be the path of least resistance. But I'm hoping to avoid having that dictate the outcome, if possible.

@mattfarina
Copy link
Member

Here's what I'm thinking.

  1. semver is used by more things than Glide/dep. We need to take them into account as well.
  2. dep is pre-alpha and may change its mind on this topic. semver shouldn't have to make changes due to that.

Given these I think I can make the package level default configurable.

@sdboyer
Copy link
Member Author

sdboyer commented Feb 7, 2017

I agree that a package-level default would be better, but that becomes global package state, and we'd run into problems if we have two systems in the same binary that expect carat-by-default, and systems that don't.

And, if we end up with on-the-fly support for glide (golang/dep#222), that'd be exactly the situation we have :(

Would it be easier if we fork semver into a new namespace, and have that be where 2.x lives? (just spitballing)

@mattfarina
Copy link
Member

@sdboyer This is more than about location. It's about tight coupling to golang/dep which is pre-alpha. As we know with pre-alpha software... there is the right to change minds on things like this.

What about a constraint factory? One of the options is to tell it what default to use.

Or, do you want to maintain a semver library tightly coupled to the development process of dep?

@sdboyer sdboyer changed the title Carat by default Caret by default Feb 8, 2017
@sdboyer
Copy link
Member Author

sdboyer commented Feb 8, 2017

It's about tight coupling to golang/dep

I suppose? I think of it more as defining this carat-by-default behavior as a clear use case, and then deciding on the mechanism by which other systems might interact with dep to ensure consistency in how semver strings get parsed.

I don't think the specifics of my last idea couple tightly, though. (Moving into a new namespace was just a thought for if we can't find a way to have these use patterns coexist). Separate caret-by-default function entry points for constraint creation and a custom ToString() printer would be a self-contained approach to the problem.

which is pre-alpha. As we know with pre-alpha software... there is the right to change minds on things like this.

This seems weird to me. It's like you're saying, "because it's pre-alpha, and things could change, so we wait and see"? Using caret-by-default is something the committee agreed on as a thing we want to at least try. There's no lack of clarity there. But it's an experimental tool, which means we only get feedback on the things we implement. There's no way to get real feedback from users on this until it's implemented.

Waiting in the lib for clear signal from the tool about requirements, and waiting in the tool for the lib to implement so that we can gather info and generate a clear signal...catch-22.

Now, if either of you want to make an argument against this in the abstract, prior to having an implementation, then by all means, please do so - but that's a discussion for the dep queue.

What about a constraint factory? One of the options is to tell it what default to use.

I think this would be equivalent to what I suggested, no?

Or, do you want to maintain a semver library tightly coupled to the development process of dep?

Just reiterating - I don't want to implement this as tight coupling; I want it as a specific feature. I'm just concerned about the complexity costs of doing so.

But to your point - no, I don't want to maintain a separate lib. I'd rather we keep it all here. But we're committed to trying this in dep, and every passing day is another day of people poking at dep, getting used to NOT having caret-by-default, which I believe is likely to shade peoples' feelings about a change simply because they've gotten used to not having it. This issue's already been open for three weeks - I need to move on it.

@sdboyer
Copy link
Member Author

sdboyer commented Feb 8, 2017

Oh - another option would be just an extra bool flag on the range type to indicate whether it should be expressed with caret default or not. That way we'd only need the additional entry point function - no ToString(). A flag wouldn't be the end of the world, as the only thing negatively impacted is == comparability, and we already didn't have that because the range type contains a slice.

@mh-cbon
Copy link
Contributor

mh-cbon commented Mar 7, 2017

Hi @sdboyer jumping in because it seems
to me the solution to set a flag on the semver package
as a global to the program is not a workable solution.

Well, i m ay get it wrong, i have not dig dep much so far.

In my understanding, there s two mode of understanding of
a semver string "x.x.x" at runtime.
Indeed, if there two projects A and B,
with A depends B
and A is a dep based project,
and B is glide based project,

When A installs,
It should read dep dependency with the ^ behavior
At some point B is imported,
the fact that it is a glide project should be denoted,
so that B dependencies such as 'x.x.x' are considered
as exact.

Without that behavior a static tree traversal of A project
could lead to wrong results. Because B might break if
its assumption of strict equality is not respected.

As a result, having a global flag seems not compatible.

What if semver is refactored to be self contained
and provides a global static facade ?

@sdboyer
Copy link
Member Author

sdboyer commented Apr 14, 2017

Ah @mh-cbon, sorry, I'm just seeing this response now - somehow I missed it in my notifications. What you're describing seems in the right ballpark, but is involving a few too many of the moving parts.

That's OK, though - I think I've been overestimating the difficulty of this. Getting this done is now formally a blocker for dep, so I'm gonna try to bang it out.

@sdboyer
Copy link
Member Author

sdboyer commented May 2, 2017

awesome, dis all done

@sdboyer sdboyer closed this as completed May 2, 2017
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

No branches or pull requests

4 participants