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

Use Freetype 2.10.2 across all pipelines #1757

Closed
wants to merge 1 commit into from
Closed

Use Freetype 2.10.2 across all pipelines #1757

wants to merge 1 commit into from

Conversation

M-Davies
Copy link
Contributor

  • Upgrade our freetype version from 2.5.3/2.9.1 to 2.10.2

Closes: #1059
Signed-off-by: Morgan Davies morgan.davies@ibm.com

@M-Davies
Copy link
Contributor Author

@sxa FYI

@karianna karianna added this to the May 2020 milestone May 21, 2020
@karianna karianna added the bug Issues that are problems in the code as reported by the community label May 21, 2020
@karianna karianna added enhancement Issues that enhance the code or documentation of the repo in any way and removed bug Issues that are problems in the code as reported by the community labels May 21, 2020
karianna
karianna requested changes May 21, 2020
View reviewed changes
Copy link
Contributor

@karianna karianna left a comment

Choose a reason for hiding this comment

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

I don't think. we can replace FreeType 2.5.3 with 2.10.2 for the older builds. It's worth trying as one-off builds to test this out. So I think the change to 2.10.2 should be made in infra? And perhaps just a comment here that for 11+ we pick up what's already on the hosts.

@M-Davies M-Davies mentioned this pull request May 22, 2020
Closed
@M-Davies
Copy link
Contributor Author

Opened an inf issue requesting to run a couple of tests with newer freetype version

@Willsparker
Copy link
Contributor

Just wondering, in what situation do we use Freetype v2.9.1 ? I can only see your PR removing the 2.5.3 versions :-)

@M-Davies
Copy link
Contributor Author

@Willsparker Since we don't specify it in the scripts, the builds pick up whichever freetype is on the machines #1059 (comment)

@Willsparker
Copy link
Contributor

It does..?
This run ( JDK11/HS ) still downloaded and built Freetype- it was 2.9.1 though, to be fair, I just can't find where in the code it decided it to be 2.9.1 as opposed to this run ( JDK11/J9 ) which opted for 2.5.3 instead...
I know this has been discussed before, but if a build is detecting a version of freetype on the machine, why get the tar and build it again?

@sxa
Copy link
Member

sxa commented May 27, 2020

And perhaps just a comment here that for 11+ we pick up what's already on the hosts.

@karianna Given that the new JDK11 build failures last night were due to freetype downloads, I don't believe that's the case (either that or we're still downloading unnecessarily ...)

The code that downloads it is at https://github.com/AdoptOpenJDK/openjdk-build/blob/master/sbin/prepareWorkspace.sh#L294

#1488 has not been implemented which would stop doing so.

@sxa
Copy link
Member

sxa commented May 27, 2020 •
edited
Loading

Freetype is set to 2.9.1 as a default in the main build script configuration: https://github.com/AdoptOpenJDK/openjdk-build/blob/adceff3deed77d862f90abb48115f49390cc8366/sbin/common/config_init.sh#L345 (Updated this comment in light of Will's next comment to avoid confusion with the wrong link being here)

However it is overridden back to 2.5.3 in various cases in windows.sh: https://github.com/AdoptOpenJDK/openjdk-build/blob/master/build-farm/platform-specific-configurations/windows.sh

@Willsparker
Copy link
Contributor

@sxa I assume you meant here? : https://github.com/AdoptOpenJDK/openjdk-build/blob/adceff3deed77d862f90abb48115f49390cc8366/sbin/common/config_init.sh#L345
:-)

@M-Davies
Copy link
Contributor Author

M-Davies commented May 28, 2020 •
edited
Loading

Thanks for all the work on adoptium/infrastructure#1337. Regarding Stewart's comment on #466 (comment), getting all builds onto Freetype 2.10.2 is not really feasible right now due to VS2017 not being supported on JDK8 builds.

We could, however, bump the following builds onto Freetype 2.10.2 since they do use TOOLCHAIN=2017:

Thoughts?

@M-Davies
Copy link
Contributor Author

M-Davies commented May 28, 2020 •
edited
Loading

Awaiting the result of https://adoptopenjdk.slack.com/archives/C09NW3L2J/p1590664035346400.

If we can't build JDK11-x86_32-Windows-Hotspot with Toolchain=2017, then this PR will need updating We can't but the question still remains if upgrading is worth the risk #1059 (comment)

Build JDK11 and upwards with freetype 2.10.2
cc65cf0 
* Freetype 2.10.2 can only be used with TOOLCHAIN=2017
* Freetype is set to use version 2.10.2 unless overriden in windows.sh

Signed-off-by: Morgan Davies <morgan.davies@ibm.com>
@sxa
Copy link
Member

sxa commented May 28, 2020

Can you switch this into draft for now to ensure that it isn't merged in the immediate future :-)

@M-Davies M-Davies marked this pull request as draft May 28, 2020 13:05
@karianna
Copy link
Contributor

Thanks for all the work on AdoptOpenJDK/openjdk-infrastructure#1337. Regarding Stewart's comment on #466 (comment), getting all builds onto Freetype 2.10.2 is not really feasible right now due to VS2017 not being supported on JDK8 builds.

We could, however, bump the following builds onto Freetype 2.10.2 since they do use TOOLCHAIN=2017:

Thoughts?

I think we want to make it all or nothing so I'll try to pull in VS2017 expert to take a look

@karianna karianna modified the milestones: May 2020, June 2020 Jun 1, 2020
@M-Davies M-Davies mentioned this pull request Jun 9, 2020
Open
@M-Davies M-Davies closed this Jun 11, 2020
@M-Davies M-Davies deleted the freetype_version branch June 11, 2020 10:06
@karianna karianna added the wontfix Issues that have been deemed as not worth or necessary to fix label Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karianna karianna karianna requested changes

Assignees
No one assigned
Labels
enhancement Issues that enhance the code or documentation of the repo in any way wontfix Issues that have been deemed as not worth or necessary to fix
Projects
None yet
Milestone
June 2020
Development

Successfully merging this pull request may close these issues.

Freetype 2.9.1 renders fonts poorly on Windows 10
4 participants
@M-Davies @Willsparker @sxa @karianna