search

d-language-server / dls

A Language Server implementation for D
http://dls.dub.pm
106 stars 15 forks source link

Allow using 1.12.x patch versions of dub #17

Closed PetarKirov closed 5 years ago

PetarKirov commented 5 years ago

The ~>1.12.0 syntax (which is short for >=1.12.0 <1.13.0) should be preferred as it allows bug fixes published as patch versions from dub to propagate to dls, while preventing breaking changes (if we assume that such may happen in a minor version). For this to work, the exact dub version should be removed from dub.selections.json, as this file overrides any flexibility in dub.json / dub.sdl.

Context: All pull requests against core D repos (dmd, druntime, phobos and dub) in addition to their own testing, are also tested on the BuildKite project tester (e.g. https://github.com/dlang/ci). It builds and tests popular projects such as yours with the combination of the master branches of all D repos and the branch of the PR. For example, a PR against dmd would combine: dmd/pull9017 + druntime/master + phobos/master + dub/master, where as a PR against druntime would combine: dmd/master + druntime/pull2376 + phobos/master + dub/master.

A DMD bug fix https://github.com/dlang/dmd/pull/9017 is blocked a by wrong cast in dub's code base. The wrong cast was removed in https://github.com/dlang/dub/pull/1600, however your project is still failing on BuildKite for https://github.com/dlang/dmd/pull/9017, because due to the way dub.json and dub.selections.json were configured, only dub 1.12.0 can be used.

With this pull request is accepted, if we publish the fix in a dub patch release (e.g. 1.12.1), when your project is build on BuildKite it will automatically use that version, instead of 1.12.0. This is necessary for https://github.com/dlang/dmd/pull/9017 to succeed on BuildKite.

To ease maintenance in the future, prefer using the version specification format which allows a range of patch versions (e.g. ~>1.12.0, ~>1.13.0, and so on) and don't commit to git changes to dub.selections.json that add a fixed version of dub.

Of course, dub.selections.json has its place - locking dependency versions is a good idea for applications that don't trust their dependencies w.r.t breaking changes, but given that no PRs against dub will be accepted if they break your project, I think it's reasonable to trust the dub release process in this case.

LaurentTreguier commented 5 years ago

I originally had tried using fixed dependencies in dub.json because I had problems with dependency resolution, but that issue is long gone, so I'll do that for other deps as well.

LaurentTreguier commented 5 years ago

Do you need a new tag for buildkite ? EDIT: nvm, I had another fix to push so I'm making one anyway

jacob-carlborg commented 5 years ago

This is the wrong fix. It’s correct to use ~> but the dub.selections.json file should be committed. The correct way to do it is Dub should get a new tag and dub.selections.json should be updated to use it.

LaurentTreguier commented 5 years ago

This is why I opened up https://github.com/dlang/ci/pull/345, to put Dub back in dub.selections.json. From what I understand, the reason for this PR is because Dub is usually released together with DMD; wouldn't tagging a new Dub version imply releasing a new DMD version ? All that for a fix that's maybe supposed to be in the next DMD version, so the whole process would be really slow for getting 2 fixes out there...

jacob-carlborg commented 5 years ago

Hmm, I see. But I don’t see why a separate tag can’t be created for Dub without doing a full release of DMD.

wilzbach commented 5 years ago

Tagged the patched dub as https://github.com/dlang/dub/releases/tag/v1.12.0-1

See also: https://github.com/dlang/dub/pull/1601#issuecomment-442506718

LaurentTreguier commented 5 years ago

I'll update as soon as it's up on code.dlang.org

wilzbach commented 5 years ago

Ah damn. I remembered SemVer wrongly:

A pre-release version MAY be denoted by appending a hyphen and a series of dot separated identifiers immediately following the patch version.

So v1.12.0-1 is considered as a prerelease version of v1.12.0 :/ I could either retag it as v1.12.1 or we just force the version for now in the dub.selections.json

LaurentTreguier commented 5 years ago

I see there is a 1.12.1-rc.1. Couldn't it be simply a new release candidate, 1.12.1-rc.2 ?

wilzbach commented 5 years ago

I see there is a 1.12.1-rc.1.

I don't see this. I only see 1.12.0-rc.1

Couldn't it be simply a new release candidate, 1.12.1-rc.2 ?

Good idea, I renamed it to v1.12.1-alpha.1 to match with the existing naming schemes. Though this would still require manual selection in the dub.selections.json file as its considered a prerelease by SemVer (and thus dub)?

LaurentTreguier commented 5 years ago

Actually my eyes were wrong, there is only 1.12.0-rc.1 indeed. I will simply force the version in the selections.

LaurentTreguier commented 5 years ago

A new tag using dub 1.12.1-alpha.1 is up