Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions jk-jeon/dragonbox/1.1.3/dragonbox_include_directory.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
diff --git a/a/CMakeLists.txt b/b/CMakeLists.txt
index a9d80b9..e0417d7 100644
--- a/a/CMakeLists.txt
+++ b/b/CMakeLists.txt
@@ -61,1 +61,1 @@
-set(dragonbox_include_directory "${CMAKE_INSTALL_INCLUDEDIR}/${dragonbox_directory}")
+set(dragonbox_include_directory "${CMAKE_INSTALL_INCLUDEDIR}")
96 changes: 96 additions & 0 deletions jk-jeon/dragonbox/1.1.3/dragonbox_llar.gox
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import (
"os"
"path/filepath"
"slices"
"strings"
)

const consumerSource = `#include <cstdlib>
#include <iostream>

#include "dragonbox/dragonbox_to_chars.h"

int main(void) {
constexpr int buffer_length = 1 +
jkj::dragonbox::max_output_string_length<jkj::dragonbox::ieee754_binary64>;
double x = 1.234;
char buffer[buffer_length];
char* end_ptr = jkj::dragonbox::to_chars(x, buffer);
end_ptr = jkj::dragonbox::to_chars_n(x, buffer);
return end_ptr == buffer ? EXIT_FAILURE : EXIT_SUCCESS;
}
`

id "jk-jeon/dragonbox"

fromVer "1.1.3"

// The Conan recipe's fPIC=True default changes the static archive, so expose
// that package-owned choice while keeping the source's subproject disabled.
Comment on lines +28 to +29

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P3] Comment conflates two unrelated build choices (fPIC vs subproject)

This comment sits on the defaults block (which only defines fPIC) but ties the fPIC choice to DRAGONBOX_ENABLE_SUBPROJECT via "so expose ... while keeping the source's subproject disabled." They're independent: fPIC -> CMAKE_POSITION_INDEPENDENT_CODE (line 53), whereas DRAGONBOX_ENABLE_SUBPROJECT (line 55) only controls building the upstream subproject/benchmark/test and is never set by the Conan recipe. Suggest stopping the comment at the fPIC rationale and moving any subproject note next to line 55.

defaults {
"fPIC": "ON",
}

filter => {
for _, value := range target.options["fPIC"] {
if value != "ON" && value != "OFF" {
return false
}
}
return true
}

onBuild ctx => {
installDir := ctx.outputDir
patch := ctx.Proj.readFile("1.1.3/dragonbox_include_directory.patch")!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P3] Header-flatten patch is a non-obvious deviation with no comment

The patch flattens upstream's versioned header install path (include/dragonbox-1.1.3/...) to include/dragonbox/... so the plain -Iinclude flag and the consumer's #include "dragonbox/dragonbox_to_chars.h" resolve. This is deliberate but unexplained at the application site. Sibling formulas document their install-layout assumptions inline (e.g. Cglm_llar.gox, Jsonc_llar.gox); a one-line comment here would keep a future maintainer from mistaking the patch for cruft.

patchPath := filepath.join(ctx.SourceDir, "_llar_dragonbox.patch")
os.writeFile(patchPath, patch, 0o644)!
git "-C", ctx.SourceDir, "apply", "--unidiff-zero", "-p2", patchPath
lastErr!
Comment on lines +45 to +49

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Patch is non-idempotent; scratch file dropped into CMake source root

onBuild writes _llar_dragonbox.patch into ctx.SourceDir (the CMake source dir passed to cmake.new on line 52) and runs git apply --unidiff-zero -p2 on every build. Two concerns:

  1. Not idempotent: git apply has no --forward/reverse guard here, so if the source tree is ever reused without a clean re-extraction (e.g. an interrupted prior build that already applied the patch), the second apply fails on the already-patched tree. Consider git apply --reverse --check (skip if already applied) or git apply --check first.
  2. Scratch file at source root: writing the patch into ctx.SourceDir can perturb CMake configure on generators that file(GLOB ... CONFIGURE_DEPENDS). Siblings keep scratch under subdirectories and don't drop loose files at the source root — writing the patch into _build (or a temp path) avoids this.


fPIC := slices.contains(target.options["fPIC"], "ON")
c := cmake.new(ctx.SourceDir, filepath.join(ctx.SourceDir, "_build"), installDir)
c.defineBool "CMAKE_POSITION_INDEPENDENT_CODE", fPIC
c.defineBool "DRAGONBOX_INSTALL_TO_CHARS", true
c.defineBool "DRAGONBOX_ENABLE_SUBPROJECT", false
c.configure
c.build
c.install

licenseDir := filepath.join(installDir, "licenses")
os.mkdirAll(licenseDir, 0o755)!
for name in []string{"LICENSE-Apache2-LLVM", "LICENSE-Boost"} {
license := os.readFile(filepath.join(ctx.SourceDir, name))!
os.writeFile(filepath.join(licenseDir, name), license, 0o644)!
}
Comment on lines +60 to +65

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P3] Hardcoded license filenames hard-fail if upstream renames them

This is the only formula in the repo doing manual license copying, and the two names are hardcoded. If either is renamed across any future fromVer range extension, os.readFile(...)! aborts the build with an opaque error. Names are correct for 1.1.3, so this is fine as-is — but confirm this licenses/ layout is a required store convention (siblings copy none); if nothing consumes it, consider dropping the block, and if it stays, note the tight coupling to exact upstream filename spelling.


flags := []string{
"-I" + filepath.join(installDir, "include"),
"-L" + filepath.join(installDir, "lib"),
"-ldragonbox_to_chars",
}
ctx.setMetadata strings.join(flags, " ")
}

onTest ctx => {
installDir := ctx.outputDir
testDir := filepath.join(ctx.SourceDir, "_llar_consumer")
os.mkdirAll(testDir, 0o755)!

consumer := filepath.join(testDir, "consumer.cpp")
os.writeFile(consumer, []byte(consumerSource), 0o644)!

binary := filepath.join(testDir, "consumer")
flags := []string{
"-std=c++17",
"-I" + filepath.join(installDir, "include"),
"-L" + filepath.join(installDir, "lib"),
"-ldragonbox_to_chars",
consumer,
"-o", binary,
}
exec "c++", flags...
lastErr!
exec binary
lastErr!
}
4 changes: 4 additions & 0 deletions jk-jeon/dragonbox/versions.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"path": "jk-jeon/dragonbox",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P3] versions.json uses 2-space indent; all siblings use tabs

jk-jeon/dragonbox/versions.json is 2-space indented, whereas json-c/json-c/versions.json, recp/cglm/versions.json, and madler/zlib/versions.json all use tabs. Match the sibling convention (tabs) for consistency and to avoid noise in any format check.

"deps": {}
}
Loading