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

Allow building on macOS with XCode 14 #634

Merged
merged 1 commit into from
Dec 7, 2022

Conversation

kgibm
Copy link
Contributor

@kgibm kgibm commented Dec 6, 2022

No description provided.

@kgibm
Copy link
Contributor Author

kgibm commented Dec 6, 2022

@pshipton

@pshipton
Copy link
Member

pshipton commented Dec 7, 2022

@keithc-ca should we be allowing the change in permissions for autogen.sh?

@keithc-ca
Copy link
Member

@keithc-ca should we be allowing the change in permissions for autogen.sh?

No, I don't think we want that change.

@pshipton
Copy link
Member

pshipton commented Dec 7, 2022

No, I don't think we want that change.

That's what I figured, what I'm not sure about is why not?

@kgibm
Copy link
Contributor Author

kgibm commented Dec 7, 2022

I just made that change for convenience. Alternatively, I could have executed the script using sh. I'm okay with reverting it.

Another question I had around the copyright check failure. I noticed that some of these files have multiple IBM copyrights. Is that correct? For example:

$ grep "Copyright.*IBM" common/autoconf/generated-configure.sh
# (c) Copyright IBM Corp. 2020, 2020 All Rights Reserved
# (c) Copyright IBM Corp. 2018, 2022 All Rights Reserved
# (c) Copyright IBM Corp. 2020, 2021 All Rights Reserved
# (c) Copyright IBM Corp. 2021, 2022 All Rights Reserved
# (c) Copyright IBM Corp. 2022, 2022 All Rights Reserved

@keithc-ca
Copy link
Member

No, I don't think we want that change.

That's what I figured, what I'm not sure about is why not?

I recall someone else proposing a similar change (in another jdk version). The documented use-case didn't require the file have execute permission - I suggested that changing the permission was something to be proposed upstream. I couldn't find documentation that applies here, but I think the same response is appropriate.

@keithc-ca
Copy link
Member

some of these files have multiple IBM copyrights. Is that correct?

Yes, that's expected (and not flagged by the copyright check job). That file in particular is generated from multiple source files, many of which have an IBM copyright notice.

@kgibm
Copy link
Contributor Author

kgibm commented Dec 7, 2022

I'll revert the permissions change.

The copyright check shows:

16:12:34  The following files were modified and have incorrect copyrights
[Pipeline] echo
16:12:34  common/autoconf/autogen.sh
16:12:34  	Does not contain a GPLv2 Classpath Exception

Is that okay?

@kgibm
Copy link
Contributor Author

kgibm commented Dec 7, 2022

Nevermind, I realize that reverting the permission change also avoids the copyright check on that file.

Signed-off-by: Kevin Grigorenko <kevin.grigorenko@us.ibm.com>
@keithc-ca
Copy link
Member

@kgibm Can you confirm that you have had a successful build using XCode 14?

@kgibm
Copy link
Contributor Author

kgibm commented Dec 7, 2022

Yes, successfully built yesterday:

$ pkgutil --pkg-info=com.apple.pkg.CLTools_Executables | grep version
version: 14.0.0.0.1.1661618636
$ ( cd build/macosx-x86_64-normal-server-release/images/j2sdk-image; ./bin/java -version)
openjdk version "1.8.0_362-internal"
OpenJDK Runtime Environment (build 1.8.0_362-internal-kevin_2022_12_06_13_39-b00)
Eclipse OpenJ9 VM (build master-504fd6a3e, JRE 1.8.0 Mac OS X amd64-64-Bit Compressed References 20221206_000000 (JIT enabled, AOT enabled)
OpenJ9   - 504fd6a3e
OMR      - 90eea450e
JCL      - a441a5a019 based on jdk8u362-b05)

And then I realized that I needed to do Linux builds for testing the OMR PR, not macOS 🤦

@keithc-ca
Copy link
Member

Jenkins compile osx jdk8

@keithc-ca keithc-ca merged commit d9472a8 into ibmruntimes:openj9 Dec 7, 2022
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

Successfully merging this pull request may close these issues.

3 participants