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.

Sign up for GitHub

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

Jump to bottom

Respect JULIA_CPU_TARGET set in Make.inc when building pkgimgs #51886

Merged
merged 1 commit into from
Oct 28, 2023
Merged

Respect JULIA_CPU_TARGET set in Make.inc when building pkgimgs #51886

merged 1 commit into from
Oct 28, 2023

Conversation

gbaraldi
Copy link
Member

No description provided.

@gbaraldi gbaraldi requested a review from vchuravy October 26, 2023 15:52
@vchuravy
Copy link
Member

This should already work... at least I tested this, but using the environment variable since this use of -C isn't really they way it's meant to be used.

@gbaraldi
Copy link
Member Author

It's how we do it when building the sysimg, and I found this causing some issues if the envar isn't set but Make.inc is

@vchuravy
Copy link
Member

I guess after #48809 this is fine, but I don't think we ever answered the question: What's the JIT target going to be if you provide a multi-versioning string as the target...

@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Oct 26, 2023
@staticfloat
Copy link
Member 

More than one command line CPU targets specified without a `--output-` flag specified

@gbaraldi
Copy link
Member Author

Right, the sysimg does --output directly, but here we create processes that do it

@vchuravy would it be more correct to do
JULIA_CPU_TARGET=* @$$(call PRINT_JULIA...

@vchuravy
Copy link
Member

Yes that's what I would have expected

@gbaraldi gbaraldi force-pushed the gb/pkg-img-cpu branch 2 times, most recently from 1950099 to 9b4ac2e Compare October 27, 2023 17:28
staticfloat
staticfloat reviewed Oct 27, 2023
View reviewed changes
pkgimage.mk Outdated Show resolved Hide resolved
@staticfloat staticfloat force-pushed the gb/pkg-img-cpu branch 2 times, most recently from 1e2cc77 to a63ae54 Compare October 27, 2023 21:35
@staticfloat
Copy link
Member

Testing locally shows that this works now; this functionality is not tested on CI, as we set the environment variables already.

@staticfloat staticfloat merged commit f106bd9 into master Oct 28, 2023
2 checks passed
@staticfloat staticfloat deleted the gb/pkg-img-cpu branch October 28, 2023 00:00
@giordano giordano removed the merge me PR is reviewed. Merge when all tests are passing label Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@staticfloat staticfloat staticfloat left review comments

@vchuravy vchuravy Awaiting requested review from vchuravy

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gbaraldi @vchuravy @staticfloat @vtjnash @giordano