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

Prepare for LLVM 18, link to local repo for scripts and patches #27178

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

alan-j-hu
Copy link
Contributor

This references the scripts and patches for my fork of opam-source-archive, just so I can see if things work on CI... If this is accepted, they will need to be changed to point to ocaml/opam-source-archive.

Context: ocaml/opam-source-archives#40

@alan-j-hu
Copy link
Contributor Author

alan-j-hu commented Dec 24, 2024

I'm getting cascading errors because of this:

Error in conf-llvm-static.18: error 46: Package is flagged "conf" but has source, install or remove instructions
Error in conf-llvm-static.18: No package source directory provided.
Error in conf-llvm-shared.18: error 46: Package is flagged "conf" but has source, install or remove instructions
Error in conf-llvm-shared.18: No package source directory provided.

This is despite the fact that this all worked on my computer testing locally, and conf-llvm.17 and prior all have build fields and were accepted. However, today when I run opam lint on conf-llvm.17, I get this error. Is this something new?

@alan-j-hu
Copy link
Contributor Author

Okay, I misunderstood the CI error message.

@alan-j-hu alan-j-hu marked this pull request as draft December 25, 2024 04:07
@alan-j-hu alan-j-hu marked this pull request as ready for review January 17, 2025 19:22
@alan-j-hu
Copy link
Contributor Author

I'm changing this PR from "draft" to "ready to review" because the opam maintainers have not seen my PR over at the opam-source-archives repository. They may have only glanced at this one, and misunderstood its status because of the "draft" state.

I need the opam maintainers to look at this PR. However, as it stands, it points at scripts on my fork of opam-source-archives. Therefore, my fork of opam-source-archives therefore needs to be merged first, so I can change the URLs on this PR.

@mseri
Copy link
Member

mseri commented Jan 17, 2025

You can link the opam-source-archive patches and files now, thanks!

@alan-j-hu
Copy link
Contributor Author

I have changed the links now.

@alan-j-hu
Copy link
Contributor Author

Hold on, don't merge this yet. I think there might be a problem with bytecode executables, which I need to check out.

@alan-j-hu
Copy link
Contributor Author

Bytecode executables were not working. (I'm not sure if they were even working before, as bytecode executables do not compile under the LLVM 14 package for a different reason.)

To fix, please merge ocaml/opam-source-archives#42 first, and then merge this.

@alan-j-hu
Copy link
Contributor Author

Argh, I spoke too soon. The bytecode executable seems to work, but I think I still made a mistake in the patch, which I'm checking out right now.

After the creation of the opam-source-archives repository, it's really, really difficult to support packages that depend on files in the opam-repository, because I need to move between two repositories and make two merge requests, leaving a lot of room for mistakes.

@alan-j-hu
Copy link
Contributor Author

Okay, ocaml/opam-source-archives#42 should be ready to merge, then this can be merged after.

Right now, I have four projects open:

  • opam-repository
  • opam-source-archives
  • llvm-project
  • My OCaml LLVM test project to make sure the LLVM package works

I need to make sure that the llvm-project test suite passes, and that the LLVM bindings also work properly when installed out-of-tree. I realized that bytecode executables didn't work when the -custom flag was passed (as it was), and set out to fix the issue, which is what the commotion was about.

@alan-j-hu
Copy link
Contributor Author

AFAIK, in LLVM 14 (the last version opam to use CMake to build), bytecode compilation wasn't working, because of a different issue that has since been fixed: https://discuss.ocaml.org/t/llvm-symbol-not-found-for-bytecode-compilation/8728. Now, I would like to pay closer attention to this package to make sure all the moving parts work.

@mseri
Copy link
Member

mseri commented Jan 25, 2025

Thanks for the effort!

By the way, you don’t need the opam-source-archive until the patch is ready. It could live on your repo or in a gist for example, giving you fast iteration, and once everything is ready and tested, we can move it to the archive

}
extra-source "AddOCaml.cmake.patch" {
src:
"https://raw.githubusercontent.com/ocaml/opam-source-archives/main/patches/llvm/AddOCaml.cmake.patch.18"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"https://raw.githubusercontent.com/ocaml/opam-source-archives/main/patches/llvm/AddOCaml.cmake.patch.18"
"https://github.com/alan-j-hu/opam-source-archives/raw/e5d49c1c15149d3a239d237634f5fc85d903718d/patches/llvm/AddOCaml.cmake.patch.18"

to iterate on the patch and test it while you are working on it, this or any other remote location would be fine

}
extra-source "AddOCaml.cmake.patch" {
src:
"https://raw.githubusercontent.com/ocaml/opam-source-archives/refs/heads/main/patches/llvm/AddOCaml.cmake.patch.18"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"https://raw.githubusercontent.com/ocaml/opam-source-archives/refs/heads/main/patches/llvm/AddOCaml.cmake.patch.18"
"https://github.com/alan-j-hu/opam-source-archives/raw/e5d49c1c15149d3a239d237634f5fc85d903718d/patches/llvm/AddOCaml.cmake.patch.18"

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