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

jpm build fails when using system SQLite library #22

Open
smartmic opened this issue Feb 11, 2024 · 9 comments
Open

jpm build fails when using system SQLite library #22

smartmic opened this issue Feb 11, 2024 · 9 comments

Comments

@smartmic
Copy link

I wanted to build the version with my system's SQLite library.
As far as I have debugged, the compilation steps do not create the output object files in the build/ directory.
If I run each cc command by hand, I will succeed.
But I have no idea and experiences with jpm why it did not create the intermediate object files.
Other jpm build steps with other packages work fine though.

Here is my issue in detail:

$ jpm --verbose build
cc -c main.c -DUSE_SYSTEM_SQLITE -DJANET_BUILD_TYPE=release  -I/usr/include/janet -I/usr/lib/janet -O2 -fPIC -o build/main.o
generating meta file build/sqlite3.meta.janet...
cc: warning: : linker input file unused because linking not done
cc: error: : linker input file not found: No such file or directory
error: error: command failed with non-zero exit code 1
  in os/execute [src/core/os.c] on line 1362
  in shell [/usr/lib/janet/jpm/shutil.janet] (tailcall) on line 143, column 5
  in <anonymous> [/usr/lib/janet/jpm/rules.janet] on line 20, column 22
error: build fail
  in pdag [/usr/lib/janet/jpm/dagbuild.janet] (tailcall) on line 79, column 23
  in run [/usr/lib/janet/jpm/cli.janet] (tailcall) on line 88, column 9
  in run-main [boot.janet] on line 3951, column 16
  in cli-main [boot.janet] on line 4104, column 17
$ # build/main.o is missing, it was not created!
$ ls build
sqlite3.meta.janet
$ # but running above compilation by hand works just fine:
$ cc -c main.c -DUSE_SYSTEM_SQLITE -DJANET_BUILD_TYPE=release  -I/usr/include/janet -I/usr/lib/janet -O2 -fPIC -o build/main.o
$ ls build
main.o  sqlite3.meta.janet
$ # now I could continue, every `cc` command only outputs its targets when run manually…
@sogaiu
Copy link

sogaiu commented Feb 12, 2024

I'm not familiar with the specifics, but I noticed mention of what seems like this scenario in the README.

It wasn't clear to me from the output above whether:

export JANET_SYSTEM_SQLITE=1

had been performed from the output above.

@smartmic
Copy link
Author

Sorry, I forgot to put it in my listing above, but, yes, export JANET_SYSTEM_SQLITE=1 was executed. If I don't do it, it will compile with the embedded SQLite3 version. This, however, works fine.

@sogaiu
Copy link

sogaiu commented Feb 12, 2024

Thanks for the clarification.

I also got similar error output.

The following diff might work depending on your setup. It worked here.

diff --git a/project.janet b/project.janet
index 14185b0..dbabdb7 100644
--- a/project.janet
+++ b/project.janet
@@ -24,8 +24,8 @@
 (if use-system-lib
   (declare-native
     :name "sqlite3"
-    :cflags (pkg-config ["sqlite3" "--cflags"]
-                        {"PKG_CONFIG_ALLOW_SYSTEM_CFLAGS" "1"})
+    :cflags ["-I/usr/include"] #(pkg-config ["sqlite3" "--cflags"]
+            #            {"PKG_CONFIG_ALLOW_SYSTEM_CFLAGS" "1"})
     :lflags (pkg-config ["sqlite3" "--libs"])
     :source @["main.c"]
     :defines {"USE_SYSTEM_SQLITE" use-system-lib})

@sogaiu
Copy link

sogaiu commented Feb 12, 2024

Another thing that might work instead of patching project.janet is:

PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 jpm --verbose build

@smartmic
Copy link
Author

@sogaiu thank you, this worked for me. I think it is worth updating project.janet and/or the Readme of this repo, what do you think about a pull request?

@sogaiu
Copy link

sogaiu commented Feb 13, 2024

I agree in principle, but I'm not sure in this case what the content of the PR ought to be.

I don't know how project.janet ought to be updated (the current content may be a reflection of an of Janet when PREFIX wasn't as well supported -- just speculation though).

Tweaking the README does seem more practical. If that's a path that makes sense to you, perhaps mentioning PKG_CONFIG_ALLOW_SYSTEM_CFLAGS for the system sqlite library use case make sense? WDYT?

@smartmic
Copy link
Author

Well, I think the current project.janet already tries to do it correctly as it specifies PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 in the struct:

:cflags (pkg-config ["sqlite3" "--cflags"]
                        {"PKG_CONFIG_ALLOW_SYSTEM_CFLAGS" "1"})

I assume this should be a janet wrapper around this shell command:

 $ PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 pkg-config --cflags sqlite3

which would give correct result:

 -I/usr/include

If my assumption is correct, than this project.janet file has a bug or is outdated. In that case, a PR would be necessary.
But I am new to Janet and absolutely not familiar with jpm, so… I do not right away know how to do it using the pkg-config wrapper in the declare-native function. On the other hand, I am quite sure that hard-coding the CFLAG as in your diff above is not acceptable. Do you know how to fix the jpm/pkg-config part above so that it works like the shell command?

@sogaiu
Copy link

sogaiu commented Feb 14, 2024

If you try with the following diff to project.janet, does it work for you?

diff --git a/project.janet b/project.janet
index 14185b0..c8a6b0a 100644
--- a/project.janet
+++ b/project.janet
@@ -11,7 +11,7 @@
 
 (defn pkg-config [what &opt env]
   (default env {})
-  (def p (os/spawn ["pkg-config" ;what] :p (merge {:out :pipe} env)))
+  (def p (os/spawn ["pkg-config" ;what] :pe (merge {:out :pipe} env)))
   (:wait p)
   (unless (zero? (p :return-code))
     (error "pkg-config failed!"))

It seems to work here.

sogaiu pushed a commit to sogaiu/sqlite3 that referenced this issue Feb 15, 2024
@smartmic
Copy link
Author

Yeah, this works for me as well. Thank you very much for the patch & your help!

bakpakin added a commit that referenced this issue Oct 25, 2024
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

No branches or pull requests

2 participants