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

Undetected git version should be a run-time error #15636

Open
eschnett opened this issue Mar 26, 2016 · 7 comments
Open

Undetected git version should be a run-time error #15636

eschnett opened this issue Mar 26, 2016 · 7 comments

Comments

@eschnett
Copy link
Contributor

I built Julia from a shallow git repository. There was a build-time warning about not being able to determine the git version number, which I saw and ignored, since I didn't know the cause for the warning. At run time, many packages (e.g. HDF5, MPI) didn't install, and I received very strange warnings from Compat.

It turns out that Compat looks at the git version number to determine what features to provide in which way, and git needs a non-shallow repo to be able to determine the version number. I didn't know either of these facts initially.

With a shallow git repo, Julia reports its version as 0.5.0-dev, which looks inconspicious. However, with a non-shallow repo, the version number is e.g. 0.5.0-dev+3263, and Compat requires this additional bit to work.

Since Compat is so fundamental, this deserves a much more visible warning, or error, and an explanation when it occurs.

I can think of several solutions:

  • Abort Julia build with an error if the git repo is shallow
  • Abort Compat package with an error and an explanation when the version number has this problem
  • Don't use version numbers to determine Compat's behaviour; instead, test which features are available
  • Create a "tag" (add a new global constant with a certain name) each time an incompatible change is made; Compat can then look for these tags instead of at the git version number
@tkelman
Copy link
Contributor

tkelman commented Mar 27, 2016

Partially a dupe of #10851. Shallow checkouts or no-git builds are fine if you're building from a release tag, but going to hit this problem anywhere else. Feature testing would generally be a better idea than version number comparisons though, if you can come up with a reliable feature test for every change that Compat and others need to worry about. Considering how rarely this comes up I don't think there will be much support for being rigorous about adding tag constants though. Post-1.0, maybe. Usually "this would be a good candidate for Compat" isn't immediately apparent.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Mar 27, 2016

much of the time, feature tests can be accomplished with a simple isdefined(Base, :foo) test. historically, I've had that rejected in favor of a test that is more explicit about the fact that this is a version-dependent change. as a compromise, I usually write my tests as if VERSION > "v0.4-" && isdefined(Base, :foo) so that there is a self-documented expected introduction version + an indication of the exact feature that changed.

@nalimilan
Copy link
Member

Maybe Compat could print a very explicit warning on load when version info is not available? (At least until we are able to replace all version checks with isdefined, if at all possible.)

@eschnett
Copy link
Contributor Author

Compat doesn't work without version information. Why not abort with an error instead?

Also, since Julia doesn't have version information, it should make this explicit in its version string. Silently reporting an incomplete (wrong) version is not a good idea. Maybe Julia could report the version as 0.5.0-dev+unknown?

@tkelman
Copy link
Contributor

tkelman commented Mar 28, 2016

Exactly what criteria do you propose using for the abort? Some way of indicating that the precise build number is unknown wouldn't be a bad idea though.

@eschnett
Copy link
Contributor Author

A version number that ends in dev instead of dev+NUMBER should cause Compat to abort. Technically that probably means that the prerelease part is dev, and the build part is empty?

@tkelman
Copy link
Contributor

tkelman commented Aug 4, 2016

#17434 may help alleviate some of your concerns here

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