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

Will you consider remove Makefile in repo. #195

Closed
Yanhao opened this issue Aug 21, 2017 · 6 comments
Closed

Will you consider remove Makefile in repo. #195

Yanhao opened this issue Aug 21, 2017 · 6 comments
Labels

Comments

@Yanhao
Copy link

Yanhao commented Aug 21, 2017

Hi, I am packaging this library for debian. I got an error while execute tests using make. But when I build and test using cmake, everything is perfect. So I wonder if you could consider remove the Makefile in repo because it's reduplicate with CMakeLists.txt and the error I've got using make.
Here is the error output:

$  cJSON git:(master) ✗ make test
./cJSON_test
Version: 1.5.7
{
        "name": "Jack (\"Bee\") Nimble",
        "format":       {
                "type": "rect",
                "width":        1920,
                "height":       1080,
                "interlace":    false,
                "frame rate":   24
        }
}
["Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"]
[[0, -1, 0], [1, 0, 0], [0, 0, 1]]
{
        "Image":        {
                "Width":        800,
                "Height":       600,
                "Title":        "View from 15th Floor",
                "Thumbnail":    {
                        "Url":  "http:/*www.example.com/image/481989943",
                        "Height":       125,
                        "Width":        "100"
                },
                "IDs":  [116, 943, 234, 38793]
        }
}
[{
                "precision":    "zip",
                "Latitude":     37.7668,
                "Longitude":    -122.3959,
                "Address":      "",
                "City": "SAN FRANCISCO",
                "State":        "CA",
                "Zip":  "94107",
                "Country":      "US"
        }, {
                "precision":    "zip",
                "Latitude":     37.371991,
                "Longitude":    -122.026,
                "Address":      "",
                "City": "SUNNYVALE",
                "State":        "CA",
                "Zip":  "94085",
                "Country":      "US"
        }]
{
        "number":       null
}
./
make: execvp: ./: Permission denied
Makefile:72: recipe for target 'test' failed
make: *** [test] Error 127

I believe the reason of the error is that there is no UTILS_TEST definition within Makefile.

@FSMaxB
Copy link
Collaborator

FSMaxB commented Aug 21, 2017

CMake is the preferred build system at this moment (maybe there will be Meson in the future). The Makefile is just there for compatibility reasons, for people that don't have CMake. It will probably removed in the future (version 2.0, if it ever happens ...)

Yes, the problem is that it tries to call an executable, that doesn't exist, because I completely moved its content into the automated testsuite. That is a bug, and I will fix it!

Note that make test is just for visual inspection. It's not actually returning a nonzero error code if it fails.

@FSMaxB FSMaxB added the bug label Aug 21, 2017
@FSMaxB
Copy link
Collaborator

FSMaxB commented Aug 21, 2017

Fixed in 1.5.8

@FSMaxB FSMaxB closed this as completed Aug 21, 2017
@Yanhao
Copy link
Author

Yanhao commented Aug 24, 2017

Note that make test is just for visual inspection. It's not actually returning a nonzero error code if it fails.

I see

Fixed in 1.5.8

Great!

@FSMaxB
Copy link
Collaborator

FSMaxB commented Aug 24, 2017

The tests that are run with CMake are proper tests though and return nonzero if they fail! Just not the Makefile.

@GitMensch
Copy link
Contributor

Sorry to pollute this a litte bit... @Yanhao wrote:

Hi, I am packaging this library for debian.

Just checked: there are multiple cJSON packages on debian, but no "plain C" - or did I miss it? The missing package is the reason that I need to build it on my Android low-end phone by hand, which is a very good reason for me that the Makefile still exists (in no way I'll put CMake on this "box").

@Yanhao
Copy link
Author

Yanhao commented Jan 18, 2019

Sorry I don't understand what do you mean about "Plain C", could you be more specific?
the cjson now is only included in debian unstable and testing distribution since buster isn't get it's first release.
the cjson is divided into two binary package: libcjson1 and libcjon-dev, the former include the following files

$  debian git:(master) dpkg -L libcjson-dev   
/.
/usr
/usr/include
/usr/include/cjson
/usr/include/cjson/cJSON.h
/usr/lib
/usr/lib/x86_64-linux-gnu
/usr/share
/usr/share/doc
/usr/share/doc/libcjson-dev
/usr/share/doc/libcjson-dev/README.md.gz
/usr/share/doc/libcjson-dev/changelog.Debian.gz
/usr/share/doc/libcjson-dev/changelog.gz
/usr/share/doc/libcjson-dev/copyright
/usr/lib/x86_64-linux-gnu/libcjson.so

and the later has these:

$  debian git:(master) dpkg -L libcjson1   
/.
/usr
/usr/lib
/usr/lib/x86_64-linux-gnu
/usr/lib/x86_64-linux-gnu/libcjson.so.1.7.10
/usr/share
/usr/share/doc
/usr/share/doc/libcjson1
/usr/share/doc/libcjson1/changelog.Debian.gz
/usr/share/doc/libcjson1/changelog.gz
/usr/share/doc/libcjson1/copyright
/usr/lib/x86_64-linux-gnu/libcjson.so.1

That's all.

and by the way, since libcjson-dev depends libcjson1, so all you need to use cjson in debian is install libcjson-dev, libcjson1 will be installed automatically then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants