-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
RFC: Simpelify packaging by splitting out collection of git version info #5095
Conversation
* Renamed BUILD_INFO to GIT_VERSION_INFO * Put the git parts of build_h.jl into version_git.jl * Made sure that an existing version_git.jl file does not get overwritten if if no git information is availible.
Hmm.. Adding another file that must not be committed has the unfortunate consequence that it will suddenly show up when you checkout another branch (or a older tagged commit). Now, if you do make clean before you change branch, you'll not get that orphaned file. |
version_git.jl.phony: | ||
@# The first check is to ensure that we don't overwrite version.git if git is unavailible. | ||
@if !( [ -f version_git.jl ] && ! [ -d ../.git ] ); then \ | ||
sh version_git.sh >> $@ \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My shell (bash
in OSX) needs a semicolon at the end of this command
@staticfloat Thanks, I'll try to implement your suggestions NO_GIT flag seems like a good suggestion. I'll also issue a warning if |
JULIA_COMMIT = $(shell git rev-parse --short=10 HEAD) | ||
else | ||
JULIA_COMMIT = "NotAvailible" | ||
endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These variables JULIA_COMMIT
and JULIA_VERSION
are almost not used. See https://gist.github.com/ivarne/8130d4cac3aa3e7772ee
It should be the same info that is used in all processes involving version and commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since JULIA_COMMIT
is only used for file naming, I think it'd be better to just set JULIA_COMMIT = JULIA_VERSION
instead of "Not Available"
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems right
… $(JULIAHOME)/.git + other minor fixes
Now we have to decide if we want to name-space the other build constants that I left in |
Let's leave the rest of |
It would be nice if it could be tested in the supported environmens on windows first. I will also add a notice in |
@vtjnash @loladiro Could one of you two test on windows? |
Also tweaked output from make
Attemt at cleaning up nighty packaging after JuliaLang/julia#5095
@nalimilan Have you tried this patch? Does it solve your problems? PS. Sorry that I did not recognize you in the other issue. |
I've grabbed the tarball from this branch, disabled my no-git patch and built the RPM, and I get an error:
The file itself contains this:
|
Thanks for trying this out. Do you really get the If so, your system probably have a echo command that does not substitute \n with newlines. I will make a fix, where i have a new echo statement for every line instead of a multiline string with newlines inside. |
Please also read my modified DISTRIBUTING.md to find out how to pregenerate the |
Cool, with the new commit it works. Why don't you build version_git.jl automatically if .git does not exist? It's not very nice to require packagers (and users installing from the tarball) to find out that very specific trick. |
The problem is that there is no way to put anything useful in the |
I also think I will try to convince the core maintainers to check in a |
We've already reached the point where tagging a release has enough steps that we should automate it. Creating |
Agreed. You should also remove all .gitignore files as they make package checkers complain. @ivarne When .git is missing, assume the VERSION file contains the correct version, and not git commit is needed. The way it currently works when you call FWIW, I've spotted a few typos in the code:
|
If the package checker complains about What do you mean I'll try to fix the spelling issues tomorrow, thanks for noticing. |
Having a target that creates a git-free tarball without .gitignore and with a generated version_git.jl file seems like the better option. |
@ivarne Currently indeed I delete .gitignore files, but I was asked to ask by Fedora people to ask Julia maintainers to produce a proper tarball free of any revision control elements; this makes life simpler for everybody. A By "automatically", I meant that people should not need to call |
My guess was that "nobody reads documentation", especially if it is long and convoluted. Therefore I made it so that it should never fail, but do the best job possible, and issue a warning if I think I miss some information. In the documentation I tell people how to do it correctly, not how they can be lazy and get a less desirable result. |
Sure. After #5181 is fixed, it would be cool to avoid printing the warning when building from a tarball, and to update DISTRIBUTING.md to explain that the git stuff is not useful in that case. |
@staticfloat While cleaning up this I found out that all the dependencies that require updated submodules also update submodules. Is there then any reason to update submodules in the main Makefile? |
That was originally done because the submodules don't know they need to update unless that is handled by the main Makefile. Conversely, the rules can be run separately, independent of the main Makefile updating them. |
@ivarne any chance this can get rebased/merged soon? I'd love to have this to streamline the ubuntu nightly process. |
…g/julia#5095"" This reverts commit 16757b1.
Merged in #5528 |
This is intended to be part of the solution to #5068. I tried to look at the submodules also, but I was confused, and hope someone else will tackle that problem.
It would be nice to get some feedback on the changes. I have only tried it on Ubuntu, because I tried to dual boot my MacBook Pro, but now I can't boot into OSX.
if no git information is availible.