-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Invalidation issue: VanillaJavaBuilder keeps stale files. #2941
Comments
The windows failure seems like a permission error, infra flake? JDK7 failure is relevant to autovalue apparently:
|
JDK7 is due to bfdad90 |
@janakdr is taking a look |
Still keeping it hot because I haven't got time to look at the windows failures. |
Looks like this is a spurious breakage. Running "bazel clean" before the build makes it go away. I think we're consuming stale jars. |
This is a moral dup of #2936 |
I am looking at Windows issue
…On Thu, May 4, 2017 at 1:20 PM, Damien Martin-Guillerez < ***@***.***> wrote:
For JDK7 linux:
http://ci.bazel.io/job/Bazel/JAVA_VERSION=1.7,PLATFORM_
NAME=ubuntu_16.04-x86_64/1467/display/redirect?page=changes
and windows:
http://ci.bazel.io/job/Bazel/JAVA_VERSION=1.8,PLATFORM_
NAME=windows-x86_64/1467/console
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#2941>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AExrX2XxPAzaFDLw_OXZFmhTUCfgrHSJks5r2bRjgaJpZM4NQiRD>
.
--
Google Germany GmbH
*Erika-Mann-Straße 33, 80636 München, Germany*
|
Reopening, we should figure out what is wrong both for windows and the correctness issue (figure out if this is an actual dup of #2936 ). I have a consistent repro. |
I can repro this locally on any incremental Java build
with bazel-0.4.5-windows-x86_64.exe (that's out bootstrap Bazel, right?)
…On Thu, May 4, 2017 at 4:08 PM, Damien Martin-Guillerez < ***@***.***> wrote:
Reopening, we should figure out what is wrong both for windows and the
correctness issue (figure out if this is an actual dup of #2936
<#2936> ). I have a consistent
repro.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2941 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AExrX5pRyB5G8xv7ZIMZJvWLKIcX630jks5r2dvJgaJpZM4NQiRD>
.
--
Google Germany GmbH
*Erika-Mann-Straße 33, 80636 München, Germany*
|
We bootstrap with bazel so yes
On Thu, May 4, 2017 at 4:10 PM Dmitry Lomov <notifications@github.com>
wrote:
… I can repro this locally on any incremental Java build
with bazel-0.4.5-windows-x86_64.exe (that's out bootstrap Bazel, right?)
On Thu, May 4, 2017 at 4:08 PM, Damien Martin-Guillerez <
***@***.***> wrote:
> Reopening, we should figure out what is wrong both for windows and the
> correctness issue (figure out if this is an actual dup of #2936
> <#2936> ). I have a consistent
> repro.
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <#2941 (comment)
>,
> or mute the thread
> <
https://github.com/notifications/unsubscribe-auth/AExrX5pRyB5G8xv7ZIMZJvWLKIcX630jks5r2dvJgaJpZM4NQiRD
>
> .
>
--
Google Germany GmbH
*Erika-Mann-Straße 33, 80636 München, Germany*
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#2941 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADjHf522UVWBnFQFMLsYw8FGK944tgkIks5r2dxcgaJpZM4NQiRD>
.
|
Also the linux bug is probably not a dupe of #2936 because it happens only with jdk7 bazel |
--strategy=Javac=standalone fixes this for 0.4.5. Now trying the latest
Bazel
On Thu, May 4, 2017 at 4:12 PM, Damien Martin-Guillerez <
notifications@github.com> wrote:
… We bootstrap with bazel so yes
On Thu, May 4, 2017 at 4:10 PM Dmitry Lomov ***@***.***>
wrote:
> I can repro this locally on any incremental Java build
> with bazel-0.4.5-windows-x86_64.exe (that's out bootstrap Bazel, right?)
>
> On Thu, May 4, 2017 at 4:08 PM, Damien Martin-Guillerez <
> ***@***.***> wrote:
>
> > Reopening, we should figure out what is wrong both for windows and the
> > correctness issue (figure out if this is an actual dup of #2936
> > <#2936> ). I have a
consistent
> > repro.
> >
> > —
> > You are receiving this because you commented.
> > Reply to this email directly, view it on GitHub
> > <#2941#
issuecomment-299195594
> >,
> > or mute the thread
> > <
> https://github.com/notifications/unsubscribe-auth/
AExrX5pRyB5G8xv7ZIMZJvWLKIcX630jks5r2dvJgaJpZM4NQiRD
> >
> > .
> >
>
>
>
> --
> Google Germany GmbH
> *Erika-Mann-Straße 33, 80636 München, Germany*
>
> —
> You are receiving this because you modified the open/close state.
>
>
> Reply to this email directly, view it on GitHub
> <#2941 (comment)
>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/
ADjHf522UVWBnFQFMLsYw8FGK944tgkIks5r2dxcgaJpZM4NQiRD>
> .
>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2941 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AExrXwup7jD8yPaWI-EfBfV2VLk7GKz0ks5r2dzWgaJpZM4NQiRD>
.
--
Google Germany GmbH
*Erika-Mann-Straße 33, 80636 München, Germany*
|
Simplest repro I have so far:
Output: However, if you don't clean in between, you get: The skyframe jar is getting stale files in it. |
Ok that sounds similar to the other bug so far.
…On Thu, May 4, 2017 at 4:23 PM Janak Ramakrishnan ***@***.***> wrote:
Simplest repro I have so far:
for br in c5672f8 bfdad90 ; do
git checkout $br
bazel clean
bazel build --define JAVA_VERSION=1.7 //src/main/java/com/google/devtools/build/skyframe:skyframe
echo "For branch $br:"
jar tf bazel-bin/src/main/java/com/google/devtools/build/skyframe/libskyframe.jar | grep SkyKey
done
Output:
For branch c5672f8
com/google/devtools/build/skyframe/SkyKey$1.class
com/google/devtools/build/skyframe/SkyKey.class
For branch bfdad90
However, if you don't clean in between, you get:
For branch c5672f8
com/google/devtools/build/skyframe/SkyKey$1.class
com/google/devtools/build/skyframe/SkyKey.class
For branch bfdad90
com/google/devtools/build/skyframe/SkyKey$1.class
com/google/devtools/build/skyframe/SkyKey.class
The skyframe jar is getting stale files in it.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#2941 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADjHf3NqRDz6XOR6hJ8OpJdJrIhmppfvks5r2d9ugaJpZM4NQiRD>
.
|
Looks like bazel-out/local-fastbuild/bin/src/main/java/com/google/devtools/build/skyframe/_javac/skyframe/libskyframe_classes has the stale SkyKey.class file. |
Is this just as silly as, javac generates .class files into the specified --classdir, and then blindly bundles everything in --classdir into a jar? So if there are any extra files in --classdir, it'll read them too? |
Yup:
|
My guess is that https://github.com/bazelbuild/bazel/blob/master/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/VanillaJavaBuilder.java#L257 should not just create the directory, it should delete any existing directory. |
Is the action not sandboxed? |
It should be but it is also using workers no?
…On Thu, May 4, 2017 at 4:56 PM Liam Miller-Cushon ***@***.***> wrote:
Is the action not sandboxed?
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#2941 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADjHfx6L-uLp1JwjrmCq4oUMgMx9SFEwks5r2ecfgaJpZM4NQiRD>
.
|
The ijar for libskyframe is the same even though a SkyKey change in
between the two changes.
On Thu, May 4, 2017 at 4:58 PM Damien Martin-Guillerez <dmarting@google.com>
wrote:
… It should be but it is also using workers no?
On Thu, May 4, 2017 at 4:56 PM Liam Miller-Cushon <
***@***.***> wrote:
> Is the action not sandboxed?
>
> —
> You are receiving this because you modified the open/close state.
> Reply to this email directly, view it on GitHub
> <#2941 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ADjHfx6L-uLp1JwjrmCq4oUMgMx9SFEwks5r2ecfgaJpZM4NQiRD>
> .
>
|
Ok this does not reproduce on latest Bazel
…On Thu, May 4, 2017 at 4:15 PM, Dmitry Lomov ***@***.***> wrote:
--strategy=Javac=standalone fixes this for 0.4.5. Now trying the latest
Bazel
On Thu, May 4, 2017 at 4:12 PM, Damien Martin-Guillerez <
***@***.***> wrote:
> We bootstrap with bazel so yes
>
> On Thu, May 4, 2017 at 4:10 PM Dmitry Lomov ***@***.***>
> wrote:
>
> > I can repro this locally on any incremental Java build
> > with bazel-0.4.5-windows-x86_64.exe (that's out bootstrap Bazel, right?)
> >
> > On Thu, May 4, 2017 at 4:08 PM, Damien Martin-Guillerez <
> > ***@***.***> wrote:
> >
> > > Reopening, we should figure out what is wrong both for windows and the
> > > correctness issue (figure out if this is an actual dup of #2936
> > > <#2936> ). I have a
> consistent
> > > repro.
> > >
> > > —
> > > You are receiving this because you commented.
> > > Reply to this email directly, view it on GitHub
> > > <#2941 (comment)
> t-299195594
> > >,
> > > or mute the thread
> > > <
> > https://github.com/notifications/unsubscribe-auth/AExrX5pRyB
> 5G8xv7ZIMZJvWLKIcX630jks5r2dvJgaJpZM4NQiRD
> > >
> > > .
> > >
> >
> >
> >
> > --
> > Google Germany GmbH
> > *Erika-Mann-Straße 33, 80636 München, Germany*
> >
> > —
> > You are receiving this because you modified the open/close state.
> >
> >
> > Reply to this email directly, view it on GitHub
> > <#2941 (comment)
> >,
> > or mute the thread
> > <https://github.com/notifications/unsubscribe-auth/ADjHf522U
> VWBnFQFMLsYw8FGK944tgkIks5r2dxcgaJpZM4NQiRD>
> > .
> >
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <#2941 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AExrXwup7jD8yPaWI-EfBfV2VLk7GKz0ks5r2dzWgaJpZM4NQiRD>
> .
>
--
Google Germany GmbH
*Erika-Mann-Straße 33, 80636 München, Germany*
--
Google Germany GmbH
*Erika-Mann-Straße 33, 80636 München, Germany*
|
@damienmg I think the _classes directory is the root of the issue. |
I don't see any warning about not sandboxing the action, even when I run explicitly with --spawn_strategy=sandboxed. The bug is still there with --spawn_strategy=standalone, of course. |
Got it. I'm on it, the fix is going to look a lot like bazel/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/SimpleJavaLibraryBuilder.java Line 116 in 9547eeb
|
@damienmg I think the ijar is blindly reading from the _classes directory.
…On Thu, May 4, 2017 at 11:07 AM, Damien Martin-Guillerez < ***@***.***> wrote:
@dslomov <https://github.com/dslomov> \o/
@janakdr <https://github.com/janakdr> ok but that won't explain why the
ijar hasn't changed.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2941 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJqlcEJNhw8mfMk6uIA4nSX7KbFFBS7Uks5r2enEgaJpZM4NQiRD>
.
|
The ijar reads the jar but the SkyKey.class should be different, no?
On Thu, May 4, 2017 at 5:09 PM Janak Ramakrishnan <notifications@github.com>
wrote:
… @damienmg I think the ijar is blindly reading from the _classes directory.
On Thu, May 4, 2017 at 11:07 AM, Damien Martin-Guillerez <
***@***.***> wrote:
> @dslomov <https://github.com/dslomov> \o/
>
> @janakdr <https://github.com/janakdr> ok but that won't explain why the
> ijar hasn't changed.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#2941 (comment)
>,
> or mute the thread
> <
https://github.com/notifications/unsubscribe-auth/AJqlcEJNhw8mfMk6uIA4nSX7KbFFBS7Uks5r2enEgaJpZM4NQiRD
>
> .
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2941 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADjHf2WXRjrl2uFlzuOKSfD5xuw4Btokks5r2eocgaJpZM4NQiRD>
.
|
No: the SkyKey class was moved to the skyframe-objects target, so the
SkyKey.class file left behind here is stale.
On Thu, May 4, 2017 at 11:10 AM, Damien Martin-Guillerez <
notifications@github.com> wrote:
… The ijar reads the jar but the SkyKey.class should be different, no?
On Thu, May 4, 2017 at 5:09 PM Janak Ramakrishnan <
***@***.***>
wrote:
> @damienmg I think the ijar is blindly reading from the _classes
directory.
>
> On Thu, May 4, 2017 at 11:07 AM, Damien Martin-Guillerez <
> ***@***.***> wrote:
>
> > @dslomov <https://github.com/dslomov> \o/
> >
> > @janakdr <https://github.com/janakdr> ok but that won't explain why
the
> > ijar hasn't changed.
> >
> > —
> > You are receiving this because you were mentioned.
> > Reply to this email directly, view it on GitHub
> > <#2941#
issuecomment-299213773
> >,
> > or mute the thread
> > <
> https://github.com/notifications/unsubscribe-auth/
AJqlcEJNhw8mfMk6uIA4nSX7KbFFBS7Uks5r2enEgaJpZM4NQiRD
> >
> > .
> >
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#2941 (comment)
>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/
ADjHf2WXRjrl2uFlzuOKSfD5xuw4Btokks5r2eocgaJpZM4NQiRD>
> .
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2941 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJqlcBwjJtn9KJyUaRTWkvGnf_ndMYDwks5r2ep6gaJpZM4NQiRD>
.
|
@dslomov are you sure that this works at HEAD with JDK7? There appears to be a discrepancy between VanillaJavaBuilder and BazelJavaBuilder, and I don't think it's gone at HEAD. |
Oh ok,
@dslomov was speaking about the windows breakage.
…On Thu, May 4, 2017 at 5:27 PM Janak Ramakrishnan ***@***.***> wrote:
@dslomov <https://github.com/dslomov> are you sure that this works at
HEAD with JDK7? There appears to be a discrepancy between
VanillaJavaBuilder and BazelJavaBuilder, and I don't think it's gone at
HEAD.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2941 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADjHf3phQl1p1oEOf1SgLltoQV0A6FRVks5r2e50gaJpZM4NQiRD>
.
|
@cushon IIUC you are working on a fix so assigning to you |
Yep, internal review cl/155081626. |
This is a temporary workaround to avoid random failures of JDK7 builds until 0.5.0 is released. Change-Id: I1323c128978190274f65018f32fd6b9afe8e681f
Should we make an announcement about this? People should know that they have to run clean before any JDK7 build if they want guaranteed correctness, no? In the case of the commit that triggered this issue, I changed a class to an interface, so the bug resulted in a compile-time error, but I could easily see it not being caught in other cases, and leading to incorrect runtime behavior. |
This is a temporary workaround to avoid random failures of JDK7 builds until 0.5.0 is released. Change-Id: I1323c128978190274f65018f32fd6b9afe8e681f
For JDK7 linux:
http://ci.bazel.io/job/Bazel/JAVA_VERSION=1.7,PLATFORM_NAME=ubuntu_16.04-x86_64/1467/display/redirect?page=changes
and windows:
http://ci.bazel.io/job/Bazel/JAVA_VERSION=1.8,PLATFORM_NAME=windows-x86_64/1467/console
The text was updated successfully, but these errors were encountered: