Skip to content

Conversation

@udesou
Copy link
Contributor

@udesou udesou commented Dec 13, 2024

Adding support for building the binding from a Makefile.

@udesou udesou changed the title Release 0.29.0 Adding Makefile support Dec 13, 2024
@udesou udesou marked this pull request as ready for review December 16, 2024 05:19
@udesou udesou requested a review from qinsoon December 16, 2024 05:19
Copy link
Member

@qinsoon qinsoon left a comment

Choose a reason for hiding this comment

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

You mentioned a valid point: with this PR, the binding repo and the VM repo record the versions of each other. We need to break this circular dependency. As Julia requires the version of its dependencies and we cannot change it, we probably want to remove the Julia version from this repo. This means, we need to change our CI as well.

@qinsoon
Copy link
Member

qinsoon commented Dec 16, 2024

You mentioned a valid point: with this PR, the binding repo and the VM repo record the versions of each other. We need to break this circular dependency. As Julia requires the version of its dependencies and we cannot change it, we probably want to remove the Julia version from this repo. This means, we need to change our CI as well.

Considering that we are under time pressure to get this upstreamed, we may prioritize getting this change approved and merged upstream.

These are what I suggest:

  1. In this PR, do whatever you need for Julia, and push directly to upstream-ready/immix. Let Julia maintainers review it.
  2. Create a master branch from dev for mmtk-julia. This is the branch that works with the upstream PR. We tag releases from master, rather than from dev.
  3. Remove the Julia version recorded in the binding in master. We only use that version for our own CI.
  4. We can remove the current CI settings for master. Once the upstream PR is merged, we will add new CI tests that pulls the Julia upstream and test with it.

We can discuss more on this tomorrow. Hopefully this can make things easier, and we do not need to figure out how the new CI should look like in this short time frame.

@udesou
Copy link
Contributor Author

udesou commented Dec 16, 2024

You mentioned a valid point: with this PR, the binding repo and the VM repo record the versions of each other. We need to break this circular dependency. As Julia requires the version of its dependencies and we cannot change it, we probably want to remove the Julia version from this repo. This means, we need to change our CI as well.

Considering that we are under time pressure to get this upstreamed, we may prioritize getting this change approved and merged upstream.

These are what I suggest:

  1. In this PR, do whatever you need for Julia, and push directly to upstream-ready/immix. Let Julia maintainers review it.
  2. Create a master branch from dev for mmtk-julia. This is the branch that works with the upstream PR. We tag releases from master, rather than from dev.
  3. Remove the Julia version recorded in the binding in master. We only use that version for our own CI.
  4. We can remove the current CI settings for master. Once the upstream PR is merged, we will add new CI tests that pulls the Julia upstream and test with it.

We can discuss more on this tomorrow. Hopefully this can make things easier, and we do not need to figure out how the new CI should look like in this short time frame.

Great idea Yi. I think that all makes sense. I'll set it all up.

@udesou udesou merged commit 46b0116 into mmtk:upstream-ready/immix Dec 17, 2024
5 checks passed
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

Successfully merging this pull request may close these issues.

2 participants