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

Enhance maximum number of supported interfaces from 2^16. #2408

Merged
merged 2 commits into from Nov 7, 2016

Conversation

Projects
None yet
@monojenkins

This comment has been minimized.

Show comment
Hide comment
@monojenkins

monojenkins Jan 8, 2016

Contributor

Hello! I'm the build bot for the Mono project. I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done. Contributors can ignore this message.

Contributor

monojenkins commented Jan 8, 2016

Hello! I'm the build bot for the Mono project. I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done. Contributors can ignore this message.

@@ -2781,7 +2781,7 @@ mono_unload_interface_id (MonoClass *klass)
* LOCKING: Acquires the classes lock.
* Returns: the new ID.
*/
static guint
static guint32
mono_get_unique_iid (MonoClass *klass)
{
int iid;

This comment has been minimized.

@Therzok

Therzok Jan 8, 2016

Member

Shouldn't this be changed to guint32?

@Therzok

Therzok Jan 8, 2016

Member

Shouldn't this be changed to guint32?

This comment has been minimized.

@tastywheattasteslikechicken

tastywheattasteslikechicken Jan 8, 2016

Contributor

What do you mean?

@tastywheattasteslikechicken

tastywheattasteslikechicken Jan 8, 2016

Contributor

What do you mean?

g_assert (iid <= 65535);
/* I've confirmed iids safe past 16 bits, however bitset code uses a signed int while testing.
* Once this changes, it should be safe for us to allow 2^32-1 interfaces, until then 2^31-2 is the max. */
g_assert (iid < INT_MAX);

This comment has been minimized.

@Therzok

Therzok Jan 8, 2016

Member

Coupled with the above change, shouldn't this be G_MAXUINT32?

@Therzok

Therzok Jan 8, 2016

Member

Coupled with the above change, shouldn't this be G_MAXUINT32?

This comment has been minimized.

@tastywheattasteslikechicken

tastywheattasteslikechicken Jan 8, 2016

Contributor

Bitset code is signed, so I left that signed rather than unsigned,

@tastywheattasteslikechicken

tastywheattasteslikechicken Jan 8, 2016

Contributor

Bitset code is signed, so I left that signed rather than unsigned,

@Therzok

This comment has been minimized.

Show comment
Hide comment
@Therzok

Therzok Jan 8, 2016

Member

Added some comments, but would like someone from the runtime team to confirm.

Member

Therzok commented Jan 8, 2016

Added some comments, but would like someone from the runtime team to confirm.

@evincarofautumn

This comment has been minimized.

Show comment
Hide comment
@evincarofautumn

evincarofautumn Jan 8, 2016

Contributor

builddeb

Contributor

evincarofautumn commented Jan 8, 2016

builddeb

@akoeplinger

This comment has been minimized.

Show comment
Hide comment
@akoeplinger

akoeplinger Jan 8, 2016

Member

builddeb

@evincarofautumn you need to make your Mono/Xamarin org membership on GitHub public or the PR builder plugin doesn't recognize you.

Member

akoeplinger commented Jan 8, 2016

builddeb

@evincarofautumn you need to make your Mono/Xamarin org membership on GitHub public or the PR builder plugin doesn't recognize you.

@tastywheattasteslikechicken

This comment has been minimized.

Show comment
Hide comment
@tastywheattasteslikechicken

tastywheattasteslikechicken Jan 12, 2016

Contributor

Just checking up, is there anything more that needs to be done here? (I haven't heard back about the CLA I submitted yet)

Contributor

tastywheattasteslikechicken commented Jan 12, 2016

Just checking up, is there anything more that needs to be done here? (I haven't heard back about the CLA I submitted yet)

@akoeplinger

This comment has been minimized.

Show comment
Hide comment
@akoeplinger

akoeplinger Jan 12, 2016

Member

@tastywheattasteslikechicken the CLA wasn't signed by your manager (Brad L.) yet.

As for the PR itself: we discussed this internally and would like to get it in, but we need to do a proper review/audit first.

Member

akoeplinger commented Jan 12, 2016

@tastywheattasteslikechicken the CLA wasn't signed by your manager (Brad L.) yet.

As for the PR itself: we discussed this internally and would like to get it in, but we need to do a proper review/audit first.

@tastywheattasteslikechicken

This comment has been minimized.

Show comment
Hide comment
@tastywheattasteslikechicken

tastywheattasteslikechicken Jan 13, 2016

Contributor

We're waiting on legal.

Contributor

tastywheattasteslikechicken commented Jan 13, 2016

We're waiting on legal.

@lewurm

This comment has been minimized.

Show comment
Hide comment
@lewurm

lewurm Mar 29, 2016

Member

@tastywheattasteslikechicken any updates on your CLA?

Member

lewurm commented Mar 29, 2016

@tastywheattasteslikechicken any updates on your CLA?

@tastywheattasteslikechicken

This comment has been minimized.

Show comment
Hide comment
@tastywheattasteslikechicken

tastywheattasteslikechicken Mar 29, 2016

Contributor

Believe it or not, I'm still waiting. About two weeks ago I was helpfully told legal are on it. What they were up to the other weeks I have no clue. I'll chase it up again today.

Contributor

tastywheattasteslikechicken commented Mar 29, 2016

Believe it or not, I'm still waiting. About two weeks ago I was helpfully told legal are on it. What they were up to the other weeks I have no clue. I'll chase it up again today.

@tastywheattasteslikechicken

This comment has been minimized.

Show comment
Hide comment
@tastywheattasteslikechicken

tastywheattasteslikechicken Apr 7, 2016

Contributor

Is it sufficient to get them to sign something releasing it under the MIT license?

Contributor

tastywheattasteslikechicken commented Apr 7, 2016

Is it sufficient to get them to sign something releasing it under the MIT license?

@akoeplinger

This comment has been minimized.

Show comment
Hide comment
@akoeplinger

akoeplinger Apr 7, 2016

Member

We're now under the .NET Foundation, so I guess you could also sign that one too (which afaik is a bit simpler): https://cla2.dotnetfoundation.org/

Member

akoeplinger commented Apr 7, 2016

We're now under the .NET Foundation, so I guess you could also sign that one too (which afaik is a bit simpler): https://cla2.dotnetfoundation.org/

@migueldeicaza

This comment has been minimized.

Show comment
Hide comment
@migueldeicaza

migueldeicaza Jul 7, 2016

Member

This probably needs to update the AOT image version.

Member

migueldeicaza commented Jul 7, 2016

This probably needs to update the AOT image version.

@monojenkins

This comment has been minimized.

Show comment
Hide comment
@monojenkins

monojenkins Jul 7, 2016

Contributor

Hello! I'm the build bot for the Mono project.

I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done.

Contributors can ignore this message.

Contributor

monojenkins commented Jul 7, 2016

Hello! I'm the build bot for the Mono project.

I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done.

Contributors can ignore this message.

@tastywheattasteslikechicken

This comment has been minimized.

Show comment
Hide comment
@tastywheattasteslikechicken

tastywheattasteslikechicken Jul 12, 2016

Contributor

Our legal department has now (2 days ago) agreed to sign the CLA. We soon will have it available to send.

Contributor

tastywheattasteslikechicken commented Jul 12, 2016

Our legal department has now (2 days ago) agreed to sign the CLA. We soon will have it available to send.

@shalupov

This comment has been minimized.

Show comment
Hide comment
@shalupov

shalupov Oct 20, 2016

Contributor

Any news on this PR? We need it in our production too.

Contributor

shalupov commented Oct 20, 2016

Any news on this PR? We need it in our production too.

@tastywheattasteslikechicken

This comment has been minimized.

Show comment
Hide comment
@tastywheattasteslikechicken

tastywheattasteslikechicken Oct 30, 2016

Contributor

The .NET Foundation one was fully online, and I've completed it (if I did it right). It wouldn't surprise me if the code was stale now after 10 months though

Contributor

tastywheattasteslikechicken commented Oct 30, 2016

The .NET Foundation one was fully online, and I've completed it (if I did it right). It wouldn't surprise me if the code was stale now after 10 months though

@akoeplinger akoeplinger reopened this Oct 30, 2016

@monojenkins

This comment has been minimized.

Show comment
Hide comment
@monojenkins

monojenkins Oct 30, 2016

Contributor

Hello! I'm the build bot for the Mono project.

I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done.

Contributors can ignore this message.

Contributor

monojenkins commented Oct 30, 2016

Hello! I'm the build bot for the Mono project.

I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done.

Contributors can ignore this message.

@dnfclas

This comment has been minimized.

Show comment
Hide comment
@dnfclas

dnfclas Oct 30, 2016

Hi @tastywheattasteslikechicken, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

dnfclas commented Oct 30, 2016

Hi @tastywheattasteslikechicken, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@akoeplinger

This comment has been minimized.

Show comment
Hide comment
@akoeplinger

akoeplinger Oct 30, 2016

Member

approve

Member

akoeplinger commented Oct 30, 2016

approve

@akoeplinger

This comment has been minimized.

Show comment
Hide comment
@akoeplinger

akoeplinger Oct 30, 2016

Member

Hmm, looks like the .NET Foundation CLA bot didn't recognize it. I'll talk to the owners on Monday about resolving that.

Member

akoeplinger commented Oct 30, 2016

Hmm, looks like the .NET Foundation CLA bot didn't recognize it. I'll talk to the owners on Monday about resolving that.

@dnfclas

This comment has been minimized.

Show comment
Hide comment
@dnfclas

dnfclas Oct 31, 2016

@tastywheattasteslikechicken, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

dnfclas commented Oct 31, 2016

@tastywheattasteslikechicken, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@dnfclas dnfclas added cla-signed and removed cla-required labels Oct 31, 2016

@kumpera kumpera merged commit 4c960e1 into mono:master Nov 7, 2016

5 of 16 checks passed

Linux AArch64 Build finished. 31982 tests run, 745 skipped, 10 failed.
Details
Linux ARMv5 soft float Build finished. 44382 tests run, 1028 skipped, 15 failed.
Details
Linux ARMv7 hard float Build finished. 38140 tests run, 904 skipped, 16 failed.
Details
Linux i386 Build finished. 44497 tests run, 1025 skipped, 1 failed.
Details
Linux x64 Build finished. 44539 tests run, 1025 skipped, 1 failed.
Details
Linux x64 FullAOT Build finished. 16417 tests run, 231 skipped, 1 failed.
Details
OS X i386 Build finished. 43261 tests run, 916 skipped, 30 failed.
Details
OS X x64 Build finished. 43261 tests run, 916 skipped, 3 failed.
Details
Tarball Build finished. No test results found.
Details
Windows i386 Build finished. 33301 tests run, 689 skipped, 3 failed.
Details
Windows x64 Build finished. 37759 tests run, 709 skipped, 16 failed.
Details
Debian package (amd64) Build finished
Details
Debian package (armel) Build finished
Details
Debian package (armhf) Build finished
Details
Debian package (i386) Build finished
Details
Debian source package Build finished
Details
@osonmez

This comment has been minimized.

Show comment
Hide comment
@osonmez

osonmez Feb 17, 2017

Hello All,
Is this fix released?

osonmez commented Feb 17, 2017

Hello All,
Is this fix released?

@akoeplinger

This comment has been minimized.

Show comment
Hide comment
@akoeplinger

akoeplinger Feb 17, 2017

Member

@osonmez it didn't make it into the upcoming Mono 4.8 release but it will be included in the one after that.

Member

akoeplinger commented Feb 17, 2017

@osonmez it didn't make it into the upcoming Mono 4.8 release but it will be included in the one after that.

@osonmez

This comment has been minimized.

Show comment
Hide comment
@osonmez

osonmez Feb 17, 2017

@akoeplinger thanks Alexander

osonmez commented Feb 17, 2017

@akoeplinger thanks Alexander

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment