Skip to content

Conversation

hoanga
Copy link

@hoanga hoanga commented Sep 11, 2025

the following changeset allows go-llvm (as well as tinygo) to compile on freebsd (tested on freebsd 14.3). without the changes applied the following error occurs when trying to compile on freebsd

$ uname -a
FreeBSD quincy 14.3-RELEASE FreeBSD 14.3-RELEASE releng/14.3-n271432-8c9ce319fef7 GENERIC amd64

$ go build ./...
# tinygo.org/x/go-llvm
./bitwriter.go:16:10: fatal error: 'llvm-c/BitWriter.h' file not found
   16 | #include "llvm-c/BitWriter.h"
      |          ^~~~~

@aykevl
Copy link
Member

aykevl commented Sep 15, 2025

While I'm not opposed to this, I'm somewhat worried about having to maintain this - and likely breaking it. Is there a way to test these things in CI, even if it's just to see whether it compiles? (I guess that might be possible by installing a FreeBSD sysroot from somewhere?)

@b0ch3nski
Copy link

I've seen other projects using https://github.com/vmactions/freebsd-vm

@hoanga
Copy link
Author

hoanga commented Sep 16, 2025

While I'm not opposed to this, I'm somewhat worried about having to maintain this - and likely breaking it. Is there a way to test these things in CI, even if it's just to see whether it compiles? (I guess that might be possible by installing a FreeBSD sysroot from somewhere?)

these are good points. i've setup an example ci workflow under a separate branch that tests using vm in freebsd approach (sample job run here). if that is acceptable as is, some options to go forward with are either:

  • update pull request with updated branch
  • fold changes into current branch with PR

i am not sure if adding a vm action might become unwieldy in everday use on this project (i imagine for some other much faster iterating projects, the extra build time might become a possible issue) but looks like it works. the current approach favors pushing all the logic into 1 vm as i am not sure if trying to spawn a matrix of jobs for vms is a great idea for github actions. i understand some other ci flavors like cirrus ci do offer freebsd support as well if that is an option that might be considered (at the cost of extra workflow definitions)

@aykevl
Copy link
Member

aykevl commented Sep 16, 2025

I have refactored the config files here: #71
With this change, FreeBSD support would just mean a few extra lines in these config files and they'd be easy to keep up to date. It's been like this in TinyGo and hasn't been an issue so far. See: https://github.com/tinygo-org/tinygo/blob/release/cgo/libclang_config_llvm20.go

@hoanga do you think that's a good idea? In that case, I can merge #71 and this PR could be remade on top of that. I can't guarantee FreeBSD will continue to work, but it's harder to accidentally mess up than if FreeBSD support still exists in separate files.

@hoanga
Copy link
Author

hoanga commented Sep 16, 2025

I have refactored the config files here: #71 With this change, FreeBSD support would just mean a few extra lines in these config files and they'd be easy to keep up to date. It's been like this in TinyGo and hasn't been an issue so far. See: https://github.com/tinygo-org/tinygo/blob/release/cgo/libclang_config_llvm20.go

@hoanga do you think that's a good idea? In that case, I can merge #71 and this PR could be remade on top of that. I can't guarantee FreeBSD will continue to work, but it's harder to accidentally mess up than if FreeBSD support still exists in separate files.

the refactoring looks a little cleaner from what i can tell. i am used to and have seen the per-os file pattern separation in go in other projects. however, if tinygo is using an alternate strategy that is working for the project, having less overall multiple conventions to decrease maintenance burden makes sense to me. the changes look okay to me and i can rebase the changes that would make sense afterwards.

my ulterior goal is to get the tinygo port updated in freebsd-ports so having less patches to add onto the ports system would be preferred.

@hoanga
Copy link
Author

hoanga commented Sep 17, 2025

i updated the freebsd-support-ci branch to incorporate changes from #71.

sample job results are here. looks okay for llvm19 and llvm20. llvm21 package does not look like it is available in package repos referenced in vm.

@b0ch3nski
Copy link

I think you could utilize the 'matrix' feature like it's done for other OS for a cleaner CI setup

strategy:
  matrix:
    llvm: [19, 20]

@hoanga
Copy link
Author

hoanga commented Sep 17, 2025

I think you could utilize the 'matrix' feature like it's done for other OS for a cleaner CI setup

strategy:
  matrix:
    llvm: [19, 20]

i am skeptical that enabling vm actions within a matrix is a good idea (see earlier comment) even if it 'cleans up' the yaml declarations.

also see this

@b0ch3nski
Copy link

also see this

@hoanga I think that this pipeline has failed because of the mistake in yaml here:

pkg install -y ${{ matrix.llvm }}

This templating would just inject a number, while you would need something like llvm${{ matrix.llvm }} (based on your commit: hoanga@ba49540)

@hoanga
Copy link
Author

hoanga commented Sep 18, 2025

also see this

@hoanga I think that this pipeline has failed because of the mistake in yaml here:

yes, you are correct. i made another attempt attempting the matrix with better results (here). the freebsd-support-ci should have the latest changes.

much appreciated on the gentle pointer to fixing an (obvious) pebkac entry error.

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.

3 participants