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

Provide def release_version(self) for BinarySuite #71

Open
olpaw opened this issue Mar 8, 2016 · 7 comments
Open

Provide def release_version(self) for BinarySuite #71

olpaw opened this issue Mar 8, 2016 · 7 comments
Assignees

Comments

@olpaw
Copy link
Member

olpaw commented Mar 8, 2016

The call to _updateGraalPropertiesFile() in mx.graal-core/mx_graal_8.py line 73 will fail with jvmci imported as binary suite because there is no definition of def release_version(self) for BinarySuite. A proper definition needs to be added.

@dougxc
Copy link
Member

dougxc commented Mar 9, 2016

Have you seen a failure? If so, under what conditions?

This should not fail as this code is only called when building the JVMCI JDK and when JVMCI is a binary suite this step is not necessary.

@olpaw
Copy link
Member Author

olpaw commented Mar 10, 2016

Have you seen a failure? If so, under what conditions?

Yes. We have a project that transitively depends on jvmci that builds (and uses) jvm-product-server-linux-amd64. If I binary deploy all transitive dependencies and then use them via MX_BINARY_SUITES='' mx build it will work fine except for binary artifact for jvm-product-server-linux-amd64. There mx tries to call _updateGraalPropertiesFile() and fails in line 73 because now jvm-product-server-linux-amd64 comes from a binary suite (which does not have def release_version(self) defined)

But when I define a dummy implementation for release_version(self) in BinarySuite everything works a expected.

@dougxc
Copy link
Member

dougxc commented Mar 10, 2016

When you execute MX_BINARY_SUITES='' mx build isn't the jvmci source suite resolved (and downloaded if necessary) bypassing the jvmci binary suite altogether?

@olpaw
Copy link
Member Author

olpaw commented Mar 10, 2016

I will send you a console log via email.

@dougxc
Copy link
Member

dougxc commented Mar 10, 2016

Sorry, I got confused and interpreted MX_BINARY_SUITES=‘’ to mean don’t use binary suites. Also, I didn’t see any failure with graal-core as the primary suite. Obviously, it has to be an imported binary suite to see the problem!

What does your dummy implementation look like? Worth creating a PR from it?

@olpaw
Copy link
Member Author

olpaw commented Mar 10, 2016

Worth creating a PR from it?

I don't think so:

def release_version(self):
    return self.version()

@dougxc
Copy link
Member

dougxc commented Mar 10, 2016

That seems reasonable to me - I don't know where else could we get version info from a binary suite?

dougxc added a commit that referenced this issue May 11, 2016
* commit 'dede2c157563190771029fd3afa35f04b7c2633d':
  Projects should reference all other projects they need and relevant resources shouldn't contain distributions
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

No branches or pull requests

2 participants