Bug 279553 - net-im/concord: New port: Discord library written in C
Summary: net-im/concord: New port: Discord library written in C
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Vladimir Druzenko
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-06-06 10:39 UTC by Souji Thenria
Modified: 2024-09-30 20:41 UTC (History)
2 users (show)

See Also:


Attachments
Diff file (3.84 KB, patch)
2024-06-06 10:39 UTC, Souji Thenria
no flags Details | Diff
Diff file (9.36 KB, patch)
2024-09-20 16:27 UTC, Souji Thenria
no flags Details | Diff
Diff file (9.48 KB, patch)
2024-09-21 10:49 UTC, Souji Thenria
no flags Details | Diff
Diff file (9.36 KB, patch)
2024-09-21 13:55 UTC, Souji Thenria
no flags Details | Diff
Diff file (9.32 KB, patch)
2024-09-21 14:43 UTC, Souji Thenria
no flags Details | Diff
Diff file (9.08 KB, patch)
2024-09-21 19:56 UTC, Souji Thenria
no flags Details | Diff
Improve port v1 (7.09 KB, patch)
2024-09-22 11:31 UTC, Vladimir Druzenko
vvd: maintainer-approval?
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Souji Thenria 2024-06-06 10:39:30 UTC
Created attachment 251249 [details]
Diff file

Hi,

This is a new port submission for concord, a low-level C library for the Discord API.

Description from the GitHub repository:
Concord is an asynchronous C99 Discord API library with minimal external dependencies, and a low-level translation of the Discord official documentation to C code.

For further reference: https://github.com/Cogmasters/concord
Comment 1 Vladimir Druzenko freebsd_committer freebsd_triage 2024-08-14 00:37:13 UTC
Is it still relevant?
Comment 2 Souji Thenria 2024-08-14 10:34:10 UTC
(In reply to Vladimir Druzenko from comment #1)
Yes, I would still like to see this library in the ports tree.
Comment 3 Vladimir Druzenko freebsd_committer freebsd_triage 2024-08-14 11:22:58 UTC
(In reply to Souji Thenria from comment #2)
What software uses it? Or are you planning to port something useful?
There are interesting examples with this library: IRC <=> Discord bot.
I would like to launch one like this.
Comment 4 Chris Hutchinson 2024-08-14 15:29:19 UTC
Thanks, Souji, for taking the time to create this port.
I'd like to suggest that the examples that come with the source,
be converted to options. So that the user can choose to
build/install some, or all of the examples that come with it.
It would be pretty easy, and would also add greater value to
this port.

Thanks, again.

--Chris
Comment 5 Vladimir Druzenko freebsd_committer freebsd_triage 2024-08-14 16:47:34 UTC
(In reply to Chris Hutchinson from comment #4)
"make examples"?
Is this command build library too?
Comment 6 Chris Hutchinson 2024-08-14 17:00:25 UTC
(In reply to Vladimir Druzenko from comment #5)
> "make examples"?
> Is this command build library too?
You can *install* examples as part of the (ports) framework.
But that simply copies them to the %%SHARE%%%%EXAMPLES%% directory.
What I meant, is to *build* some, or all the examples as an
option. Like the "bot" you already mentioned. It'd be really
easy, and makes the port quite a bit more valuable. Don't you think?
Comment 7 Vladimir Druzenko freebsd_committer freebsd_triage 2024-08-14 18:07:45 UTC
(In reply to Chris Hutchinson from comment #6)
My question was: what is the command to build the examples?
Comment 8 Chris Hutchinson 2024-08-14 20:29:48 UTC
(In reply to Vladimir Druzenko from comment #7)
Sure. Sorry.
% make examples.

But after closer examination. I see it'd probably be smarter
to simply copy the examples, in much the same way we do DOC's.
As most of the examples require tweaking/personalization prior
to building. This would allow building any of them, and running
them out of ~ , or ~/bin.
Comment 9 Souji Thenria 2024-08-14 20:40:28 UTC
(In reply to Chris Hutchinson from comment #8)
Hi Chris,

Thanks for the suggestion; I can look into making the examples available. 

As you already pointed out, I, too, think that making the .c files with the examples available makes more sense than compiling them. People using the library might want to look at the example code and compile it themselves rather than having a binary.

I won't be able to make that change this month, but I should be able to apply the change in September.
Comment 10 Vladimir Druzenko freebsd_committer freebsd_triage 2024-08-15 01:00:52 UTC
Don't need:
MASTER_SITES=	https://github.com/Cogmasters/concord/
if used "USE_GITHUB=	yes".

Also don't need "GH_PROJECT=	concord" - by default GH_PROJECT=${PORTNAME}.

And this too:
GH_TAGNAME=	${DISTVERSIONPREFIX}${PORTVERSION}
it's default value.
Comment 11 Vladimir Druzenko freebsd_committer freebsd_triage 2024-09-20 15:47:25 UTC
(In reply to Souji Thenria from comment #9)
ping
Comment 12 Souji Thenria 2024-09-20 16:03:42 UTC
(In reply to Vladimir Druzenko from comment #11)
pong

I'm sitting on it right now, actually, and I'm almost done (I hope).
Comment 13 Souji Thenria 2024-09-20 16:27:41 UTC
Created attachment 253693 [details]
Diff file
Comment 14 Souji Thenria 2024-09-20 16:28:21 UTC
Sorry for the delay...

As Vladimir proposed, I have removed some of the redundant options. Moreover, as Chris requested, I added the examples to the port.

Furthermore, I added the other build targets and options (compiler flags) mentioned in the project's README that others might want to use.
Comment 15 Vladimir Druzenko freebsd_committer freebsd_triage 2024-09-20 17:30:06 UTC
Maybe primary category net-im?
CATEGORIES= net-im devel
Comment 16 Souji Thenria 2024-09-20 18:43:08 UTC
(In reply to Vladimir Druzenko from comment #15)

It looks like it would fit quite well into net-im, too. When I change the primary category to net-im, I also need to move the port in the ports tree to that directory, don't I?
Comment 17 Vladimir Druzenko freebsd_committer freebsd_triage 2024-09-20 19:45:47 UTC
(In reply to Souji Thenria from comment #16)
Ye.

Align values after variables with tabs.
For example:
PORTNAME=concord
PORTNAME=       concord

Check other ports.

Also use portclippy from ports-mgmt/portfmt and ports-mgmt/portlint for test port's "quality".
Comment 18 Souji Thenria 2024-09-21 10:49:01 UTC
Created attachment 253716 [details]
Diff file
Comment 19 Souji Thenria 2024-09-21 10:52:24 UTC
(In reply to Vladimir Druzenko from comment #17)

I changed it, and it passes portlint as well as portclippy.

I have never heard about portclippy; it's a great tool, thanks.
Comment 20 Vladimir Druzenko freebsd_committer freebsd_triage 2024-09-21 12:06:19 UTC
(In reply to Souji Thenria from comment #19)
1. Why OPTIONS_DEFAULT=STATIC ?
SHARED is preferred always.

2. I think this can work:
SIGINTCATCH_CFLAGS= -DCCORD_SIGINTCATCH
instead of this:
.if ${PORT_OPTIONS:MSIGINTCATCH}
CFLAGS+=	-DCCORD_SIGINTCATCH
.endif

Same for DEBUG_WEBSOCKETS and DEBUG_HTTP.

3. I'll check MAKE_TARGET - maybe possible to do better.
Comment 21 Souji Thenria 2024-09-21 12:20:37 UTC
(In reply to Vladimir Druzenko from comment #20)
1. 'static' is used in the libraries Makefile for the default target:
https://github.com/Cogmasters/concord/blob/5908b4923e13914c898b6dc187ce387b9218bccc/Makefile#L23C6-L23C12

But I can change it to SHARED for the port if preferred.

2. I'll look into it and change it if it works.

3. That would be great. I admit it is not pretty; I didn't find a better method which works.
Comment 22 Vladimir Druzenko freebsd_committer freebsd_triage 2024-09-21 12:25:49 UTC
(In reply to Vladimir Druzenko from comment #20)
2. Or maybe:
SIGINTCATCH_MAKE_ARGS= CCORD_SIGINTCATCH
Comment 23 Souji Thenria 2024-09-21 13:17:02 UTC
(In reply to Vladimir Druzenko from comment #22)
2. "*_CFLAGS=..." works
Comment 24 Souji Thenria 2024-09-21 13:55:04 UTC
Created attachment 253721 [details]
Diff file
Comment 25 Souji Thenria 2024-09-21 14:01:32 UTC
(In reply to Vladimir Druzenko from comment #20)
1. I kept it STATIC for now, but I would ask the devs why they made that the default.

2. As mentioned earlier, it worked with CFLAGS.

3. Now, I use MAKE_ARGS for that. It works and looks better.


Note: portclippy would change the order of the "Options helpers", but I would rather not mix the CFLAGS and MARKE_ARGS variables.
Comment 26 Vladimir Druzenko freebsd_committer freebsd_triage 2024-09-21 14:13:33 UTC
(In reply to Souji Thenria from comment #25)
3. Nice catch!
> Note: portclippy would change the order of the "Options helpers", but I would rather not mix the CFLAGS and MARKE_ARGS variables.
IMHO, better sort variables alphabetically - any new developer (who don't know why you use custom order) easier find options in Makefile and easier can update port.

4. Default value for DEBUG_DESC (defined in Mk/bsd.options.desc.mk) is:
DEBUG_DESC?=            Build with debugging support
your value is:
DEBUG_DESC=		Build with debug flags
You can keep your value or you can remove DEBUG_DESC from Makefile and use default from ports framework.
Comment 27 Souji Thenria 2024-09-21 14:42:57 UTC
(In reply to Vladimir Druzenko from comment #26)
I like sorting by function better, but I see the reason for making it uniform across ports.

1. I changed it to SHARED. The devs weren't too sure either why it isn't the default.

4. That's nice, I changed it.
Comment 28 Souji Thenria 2024-09-21 14:43:17 UTC
Created attachment 253722 [details]
Diff file
Comment 29 Vladimir Druzenko freebsd_committer freebsd_triage 2024-09-21 16:33:47 UTC
5. "EXAMPLESDIR=   ${PREFIX}/share/examples/${PORTNAME}" is default value defined in Mk/bsd.port.mk - just remove it.

6. Use REINPLACE_CMD:
-        cd ${WRKSRC}/examples/ && ${SED} -i '' 's/#include "\([^"]*\)\.h"/#include <concord\/\1.h>/g' *.c
-        cd ${WRKSRC}/examples/ && ${SED} -i '' 's|\.\./config\.json|\.\/config\.json|g' *.c
+        ${REINPLACE_CMD} -e 's|#include "\([^"]*\)\.h"|#include <concord/\1.h>|g; \
+                s|\.\./config\.json|./config.json|g' ${WRKSRC}/examples/*.c

7.
DEBUG_VARS=                     ALL_TARGET=debug
DEBUG_HTTP_CFLAGS=              -DCCORD_DEBUG_HTTP
DEBUG_WEBSOCKETS_CFLAGS=        -DCCORD_DEBUG_WEBSOCKETS
SHARED_VARS=                    ALL_TARGET=shared
SIGINTCATCH_CFLAGS=             -DCCORD_SIGINTCATCH
STATIC_VARS=                    ALL_TARGET=static
And remove this:
do-build:
       cd ${WRKSRC} && ${SETENV} ${MAKE_ENV} ${MAKE} ${MAKE_ARGS}

8. Trying to do something with do-install…
Comment 30 Vladimir Druzenko freebsd_committer freebsd_triage 2024-09-21 17:20:00 UTC
8. Add:
MAKE_ARGS=      DESTINCLUDE_DIR=${STAGEDIR}${PREFIX}/include/concord \
                DESTLIBDIR=${STAGEDIR}${PREFIX}/lib
and remove all do-install section.
Comment 31 Vladimir Druzenko freebsd_committer freebsd_triage 2024-09-21 17:20:39 UTC
Why SIGINTCATCH isn't default?
Comment 32 Vladimir Druzenko freebsd_committer freebsd_triage 2024-09-21 17:30:46 UTC
6. Why do you use "g" key or sed? #include always 1 per line.
Replace "../config.json" with "./config.json" - why we need "./" before "config.json"?
Why we can't replace with just "config.json"?

9. We can remove line: ${INSTALL} -d ${STAGEDIR}${EXAMPLESDIR}

Tested build with all my changes in poudriere and live system 14.1 amd64.
Comment 33 Souji Thenria 2024-09-21 19:56:59 UTC
Created attachment 253732 [details]
Diff file
Comment 34 Souji Thenria 2024-09-21 19:57:25 UTC
(In reply to Vladimir Druzenko from comment #32)
5. Done.
6. Done.
7. Done.
8. Done.
9. Done.

I added your changes and tested again on 14.1-STABLE, and it works for me, too.

Regarding SIGNINTCATCH: When this is specified, the library will handle SIGINT and shut down gracefully when SIGINT is received. The library also disables this by default, and I went by the library's configurations when creating the options for that port.

Furthermore, in my opinion, the library should not set the signal handler; the programmer should decide what signals need to be handled and write the handlers accordingly. But in the end, it does not hurt anybody to enable it, so if you think it would be advantageous to have it as a default, I can add it.
Comment 35 Vladimir Druzenko freebsd_committer freebsd_triage 2024-09-21 20:40:19 UTC
(In reply to Souji Thenria from comment #34)
About SIGNINTCATCH you are right - it's library.
Comment 36 Souji Thenria 2024-09-21 21:25:55 UTC
(In reply to Vladimir Druzenko from comment #35)
Do you think removing that option from the port makes more sense?

Oh, and thank you very much for your help and suggestions earlier :)
Comment 37 Vladimir Druzenko freebsd_committer freebsd_triage 2024-09-21 21:33:35 UTC
(In reply to Souji Thenria from comment #36)
No, keep as non-default.

BTW, I still have a question: are you planning to port something useful using this port?
Comment 38 commit-hook freebsd_committer freebsd_triage 2024-09-21 21:43:25 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=97d2f200cd1f21f86c92d2644f563a86a89d5c65

commit 97d2f200cd1f21f86c92d2644f563a86a89d5c65
Author:     Souji Thenria <mail@souji-thenria.net>
AuthorDate: 2024-09-21 21:40:55 +0000
Commit:     Vladimir Druzenko <vvd@FreeBSD.org>
CommitDate: 2024-09-21 21:42:33 +0000

    net-im/concord: New port: Discord library written in C

    Concord is an asynchronous C99 Discord API library with minimal external
    dependencies, and a low-level translation of the Discord official
    documentation to C code.
    https://github.com/Cogmasters/concord

    PR:     279553

 net-im/Makefile                                 |  1 +
 net-im/concord/Makefile (new)                   | 52 +++++++++++++++
 net-im/concord/distinfo (new)                   |  3 +
 net-im/concord/files/Makefile.examples (new)    | 48 ++++++++++++++
 net-im/concord/files/config.json.examples (new) | 21 +++++++
 net-im/concord/files/patch-src_Makefile (new)   | 10 +++
 net-im/concord/pkg-descr (new)                  |  3 +
 net-im/concord/pkg-plist (new)                  | 84 +++++++++++++++++++++++++
 8 files changed, 222 insertions(+)
Comment 39 Souji Thenria 2024-09-21 21:53:30 UTC
(In reply to Vladimir Druzenko from comment #37)
Ok.

As of right now, no (I don't have anything other than some personal projects). However, I plan to write an IRC-Discord bridge in the near future and other tools to monitor various systems and system stats and make those available in Discord. If the community is interested, I'd like to add those to the ports tree, too.
Comment 40 Vladimir Druzenko freebsd_committer freebsd_triage 2024-09-21 21:56:22 UTC
(In reply to Souji Thenria from comment #39)
I'm interesting in software "IRC-Discord bridge".
Comment 41 Vladimir Druzenko freebsd_committer freebsd_triage 2024-09-22 11:31:59 UTC
Created attachment 253738 [details]
Improve port v1

Check my patch.
Comment 42 Souji Thenria 2024-09-24 00:06:17 UTC
(In reply to Vladimir Druzenko from comment #41)
It looks good so far, thank you!

But there is one thing I'm not sure about. Why would you distribute the examples as precompiled binaries in addition to the source code? Since all the requirements to compile the examples are met after installing the port, and only the execution of the Makefile is needed, I feel like the precompiled examples are redundant.
Comment 43 Vladimir Druzenko freebsd_committer freebsd_triage 2024-09-28 17:23:07 UTC
(In reply to Souji Thenria from comment #42)
I'm not sure which way is right. But maybe someone will find precompiled binaries useful without having to compile them themselves.
Comment 44 Souji Thenria 2024-09-30 16:44:11 UTC
(In reply to Vladimir Druzenko from comment #43)
Yeah, maybe; in that case, let's go with it. Thanks again!
Comment 45 commit-hook freebsd_committer freebsd_triage 2024-09-30 20:40:58 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=27e816ac013a9cb938481342c26d3398b04453f9

commit 27e816ac013a9cb938481342c26d3398b04453f9
Author:     Vladimir Druzenko <vvd@FreeBSD.org>
AuthorDate: 2024-09-30 20:31:42 +0000
Commit:     Vladimir Druzenko <vvd@FreeBSD.org>
CommitDate: 2024-09-30 20:40:33 +0000

    net-im/concord: Improve port

    - Add USE_LDCONFIG - port installs .so library.
    - Allow build both shared and static libraries at same time.
    - Prebuild examples.

    PR:             279553
    Approved by:    Souji Thenria <mail@souji-thenria.net>

 net-im/concord/Makefile                            | 35 +++++++++++-----------
 net-im/concord/files/patch-Makefile (new)          | 11 +++++++
 net-im/concord/files/patch-examples_Makefile (new) | 11 +++++++
 net-im/concord/files/patch-src_Makefile            |  7 +++--
 net-im/concord/pkg-plist                           | 30 +++++++++++++++++--
 5 files changed, 71 insertions(+), 23 deletions(-)
Comment 46 Vladimir Druzenko freebsd_committer freebsd_triage 2024-09-30 20:41:21 UTC
Committed, thanks.