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

[20.2] Backports included in Mandrel/20.1 #117

Merged
merged 13 commits into from
Sep 3, 2020

Conversation

adinn and others added 12 commits September 3, 2020 00:24
* Before this patch,
the image would contain zone rules cache from the JVM computing the image.
* Side effects here,
such as class or source cache lookups,
were affecting its contents and leading to error when writing image.
* With this change,
we guarantee that the zone rules cache in the image is a brand new instance,
separate from the one in the JVM computing the image.
Since inlined methods may be defined in a different file than the one
defining a compiled method, the cache paths of all the source files
contributing code to the compiled method need to be set as the
compilation directory to make gdb work.

E.g. If:
1. `foo` invokes `bar` and `baz`
2. `bar` and `baz` get inlined in `foo`
3. `foo`'s source file is cached in `sources/src`
4. `bar`'s source file is cached in `sources/jdk`
5. `baz`'s source file is cached in `sources/graal`

then the compilation directory in `foo`'s compilation unit needs to be
set to `sources/src:sources/jdk:sources/graal`
Partial revert of "Adds DW_AT_comp_dir attribute to debug info"
commit 4f231ab.

Closes oracle#2751
Since graal is sensitive to JDK changes printing the build number along
with the java version, makes issue reporting and debugging a bit easier.
@zakkak zakkak added the backport label Sep 2, 2020
@zakkak zakkak added this to the 20.2.0.0.Alpha1 milestone Sep 2, 2020
@zakkak zakkak added this to In progress in Mandrel via automation Sep 2, 2020
@zakkak zakkak self-assigned this Sep 2, 2020
With JDK-8236548 in OpenJDK 11 an issue running
a generated native image may arise on programs using
TimeZone.getDisplayName(). With JDK-8236548 this might
become more noticeable since it now occurs on the
English locale too (no caching of English locales
happens any more).

Since all time zone data is included by default now,
also include the TimeZoneNames resource bundle by
default.

Closes: graalvm#106
Mandrel automation moved this from In progress to Reviewer approved Sep 3, 2020
Copy link
Collaborator

@jerboaa jerboaa left a comment

Choose a reason for hiding this comment

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

OK. I'd suggest to add some automation for forward-branch merges (20.1 => 20.2 in this case) which opens those PRs automatically.

@zakkak
Copy link
Collaborator Author

zakkak commented Sep 3, 2020

OK. I'd suggest to add some automation for forward-branch merges (20.1 => 20.2 in this case) which opens those PRs automatically.

+1 but it will require some effort.

Doing it manually I found out that:

  1. There are commits in 20.1 that were never pushed to master and we don't want to bring them to 20.2 (these are mostly version bumps of dependencies and such, so in theory we could detect and skip them)
  2. git log --cherry sometimes fails to identify identical commits (not sure how to overcome this)
  3. We need a way to link cherry-picked commits to the original commit and then to the corresponding upstream PR. For the first part there's probably some git magic, and for the second part there seems to be some API support .

@zakkak
Copy link
Collaborator Author

zakkak commented Sep 3, 2020

^^ That's for the initial sync however. For new PRs it should be easier.

@zakkak zakkak merged commit 9e29645 into graalvm:mandrel/20.2 Sep 3, 2020
Mandrel automation moved this from Reviewer approved to Done Sep 3, 2020
@zakkak zakkak deleted the mandrel/20.2-backports branch September 3, 2020 09:29
@galderz
Copy link
Collaborator

galderz commented Sep 4, 2020

Thx @zakkak for handling this.

There are commits in 20.1 that were never pushed to master and we don't want to bring them to 20.2 (these are mostly version bumps of dependencies and such, so in theory we could detect and skip them)

+1. The only dependency we had was ASM and that is gone now. So we can skip those.

We need a way to link cherry-picked commits to the original commit and then to the corresponding upstream PR. For the first part there's probably some git magic, and for the second part there seems to be some API support .

Why do you need the link to the upstream PR?

@zakkak
Copy link
Collaborator Author

zakkak commented Sep 4, 2020

+1. The only dependency we had was ASM and that is gone now. So we can skip those.

The dependency updates were pushed by Oracle not us, so these commits were not actually backported, just commits in 20.1 that are not in 20.2. So we essentially need to get the commits that are both on 20.1 and master but not on 20.2 (we will need to work on a new command combo for this :) )

We need a way to link cherry-picked commits to the original commit and then to the corresponding upstream PR. For the first part there's probably some git magic, and for the second part there seems to be some API support .

Why do you need the link to the upstream PR?

IMO This makes reviewing and keeping track of what's going on easier. Additionally at some point we should create a changelog and I think linking to PR's instead of commits is better (less verbose).

@zakkak zakkak modified the milestones: 20.2.0.0.Alpha1, 20.2.0.0.Final Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants