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

Ease building with custom includedir. #7316

Merged
merged 3 commits into from
Jun 22, 2014
Merged

Ease building with custom includedir. #7316

merged 3 commits into from
Jun 22, 2014

Conversation

crayxt
Copy link
Contributor

@crayxt crayxt commented Jun 19, 2014

If you specify build_includedir to be inside of $(build_prefix)/usr and not in $(build_prefix)/include then include files of gmp and LLVM are not installed to build_includedir but to $(build_prefix)/include.
The only thing missing is specifying pcre.h header file for pcre_h.jl. I am still thinking on it.

@crayxt
Copy link
Contributor Author

crayxt commented Jun 19, 2014

Cast @nalimilan

@ViralBShah
Copy link
Member

Cc: @staticfloat

@staticfloat
Copy link
Sponsor Member

@crayxt Could you give us a little more info as to what the idea behind this PR is? I think I agree with most of these changes, (except perhaps for the LLVM_CONFIG changes; what are those changes for?) but it would help to know why these changes are necessary for you.

@crayxt
Copy link
Contributor Author

crayxt commented Jun 20, 2014

@staticfloat I apologize for not providing good explanation from the beginning.
The main idea is when you specify custom $(build_includedir), most of the packages from deps directory follow that location, except for llvm and gmp. They store own header files into hardcoded path $(build_prefix)/include. So I tried to make all deps packages behave the same way.

As for llvm-config change, it addresses the similar issue of hardcoded path, this time for binary files. If I change $(build_bindir) to my own value, the old location of llvm-config would be incorrect, but $(build_bindir)/llvm-config remains correct when USE_SYSTEM_LLVM=0.

@nalimilan
Copy link
Member

Makes much sense, but I think @staticfloat was curious about the situation in which you need it. It's always interesting to understand changes.

BTW, if the llvm-config change is about binaries, you should make the commit message more general.

@staticfloat
Copy link
Sponsor Member

Ah, I see; LLVMROOT is just $(build_prefix).

These are all good changes. It sounds like your pcre.h changes are not complete though. Please let me know when the changes are complete so I can test.

@crayxt
Copy link
Contributor Author

crayxt commented Jun 21, 2014

@staticfloat Pull request updated. I couldn't find a better way other than using ifeq switch.

@staticfloat
Copy link
Sponsor Member

I don't think your sed invocation works right on OSX. When I run this branch, I get:

sed: 1: "Makefile.in": invalid command code M
make[2]: *** [gmp-5.1.3/config.status] Error 1
make[1]: *** [julia-release] Error 2
make: *** [release] Error 2
make build_includedir=$(pwd)/usr/include2 took 05:37:07

I think it's because of these lines which unfortunately don't work on OSX because sed -i requires a file extension afterward. I think if you just change this to -ibackup or something, it should work.

@tkelman
Copy link
Contributor

tkelman commented Jun 22, 2014

Do you really need to use sed there? Can't an extra configure argument accomplish the same thing?

@crayxt
Copy link
Contributor Author

crayxt commented Jun 22, 2014

@tkelman I think you are right, I will check

@crayxt
Copy link
Contributor Author

crayxt commented Jun 22, 2014

@staticfloat Pull request updated. I suppose there is no reason to wait for Travis results, as it uses system libs. Sadly, I don't have OSX to check this.
@tkelman I couldn't make it work with only supplying flags to configure script.

@$(call PRINT_PERL, $(CPP) -dM $(shell $(PCRE_CONFIG) --prefix)/include/pcre.h | perl -nle '/^\s*#define\s+PCRE_(\w*)\s*\(?($(PCRE_CONST))\)?\s*$$/ and print "const $$1 = uint32($$2)"' | sort > $@)
else
@$(call PRINT_PERL, $(CPP) -dM $(build_includedir)/pcre.h | perl -nle '/^\s*#define\s+PCRE_(\w*)\s*\(?($(PCRE_CONST))\)?\s*$$/ and print "const $$1 = uint32($$2)"' | sort > $@)
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How delightfully ironic that we use these convoluted regexes to patch PCRE.

@staticfloat
Copy link
Sponsor Member

Oddly enough, I couldn't find the magic invocations to configure or make to get it to put the header files where we wanted them either. This is good for me, I'm going to merge.

staticfloat added a commit that referenced this pull request Jun 22, 2014
Ease building with custom includedir.
@staticfloat staticfloat merged commit 98deb70 into JuliaLang:master Jun 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:building Build system, or building Julia or its dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants