Skip to content

Build fix - #3

Merged
vkuttyp merged 5 commits into
vkuttyp:mainfrom
FoxClock:BuildFix
Feb 22, 2026
Merged

Build fix#3
vkuttyp merged 5 commits into
vkuttyp:mainfrom
FoxClock:BuildFix

Conversation

@FoxClock

Copy link
Copy Markdown
Contributor

Changes

Sources/CFreeTDS/include/CFreeTDS.h

Replaced hardcoded Cellar paths (which broke on every brew upgrade) with Homebrew's stable opt symlink. Linux continues to use system resolution via angle bracket includes.

  • Now uses /usr/local/opt/freetds/include/ on macOS — a stable symlink maintained by Homebrew regardless of installed version
  • Linux path unchanged, resolved via pkg-config

Package.swift

Updated linker settings to support both Intel and Apple Silicon Macs by providing both Homebrew prefix locations. The compiler silently ignores paths that don't exist, so providing both is safe.

  • Added -L/opt/homebrew/opt/freetds/lib for Apple Silicon
  • Retained -L/usr/local/opt/freetds/lib for Intel
  • No changes to Linux configuration — handled by pkgConfig: "freetds" and the apt provider on the systemLibrary target

Build prerequisites (macOS):

brew install pkg-config
brew install freetds

This now allows for the project to compile and build on my intel mac as well as ARM Macs

@FoxClock

Copy link
Copy Markdown
Contributor Author

Project wouldn't compile on intel mac. The header file would not resolve correctly. Only by editing this file was I able to get the file to resolve the FreeTDS includes.

The inclusion of the brew install pkg-config may resolve this on some systems, however, the issue is that on my system it did not resolve as the homebrew install location was not in its search path.

To resolve, I used the simlink location for the freetds install. However, this seems to have broken the CI pipeline you have created.

To address this, without being able to re-test. I have added a compiler exception for x86 processors, where it will include the symlink but default to your configuration otherwise. I don't like this as it adds complexity.

@vkuttyp vkuttyp left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for catching this — Intel Mac support is a real bug that needs fixing. The direction is right, but I'd like to address it more cleanly rather than hardcoding either path or using #if arch.
The cleanest fix is to provide both include paths via unsafeFlags in Package.swift and go back to angle-bracket includes in the header — the compiler will silently ignore whichever path doesn't exist on the current machine:

// Package.swift — CSybdb target cSettings:
cSettings: [
    .unsafeFlags([
        "-I/opt/homebrew/opt/freetds/include",  // Apple Silicon
        "-I/usr/local/opt/freetds/include",     // Intel
    ]),
],
// CSybdb.h — back to angle brackets, no hardcoded paths:
#include <sybfront.h>
#include <sybdb.h>

This way the header works on both architectures, no #if arch needed, and it also works on Linux without changes. Could you update the PR with this approach?

…in both X86 and ARM64 locations.

Added all build targets to test target as well, so the program compiles under test and build instructions.

@hfoxwell hfoxwell left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thank you for your feedback, you hit the nail on the head with the added compiler flags.

I added them in and simplified the C header imports, so, now they should work across both systems.

I also added the build parameters to the testing target. This way the project will now compile under both

@vkuttyp vkuttyp left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Great work — this is the right fix. Clean header with angle-bracket includes, both Homebrew paths passed via -Xcc in swiftSettings, and properly gated with .when(platforms: [.macOS]). This will work correctly on both Apple Silicon and Intel without any hardcoded assumptions.
Minor suggestion for a follow-up: the cSettings/swiftSettings/linkerSettings blocks on the test target are duplicates of what the main target already provides — the test target inherits these through its dependency on SQLClientSwift. Feel free to strip those from the test target to keep Package.swift cleaner.
Approving — please make sure CI passes before we merge.

@vkuttyp
vkuttyp merged commit b05b0df into vkuttyp:main Feb 22, 2026
1 check 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.

3 participants