Skip to content

Conversation

samuelvenable
Copy link

Very special thanks to @k3nrap from the FreeBSD Discord for all the hard work that went into this! :D

Very special thanks to @k3nrap from the FreeBSD Discord for all the hard work that went into this! :D
@@ -0,0 +1,3 @@
bin/filedialogs
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this some sort of demo program?

Copy link
Author

Choose a reason for hiding this comment

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

it's a demo program but also a command line interface, serves a similar purpose as zenity or kdialog

@@ -0,0 +1,65 @@
PORTNAME= sdl3-imgui-filedialogs
DISTVERSIONPREFIX= v
DISTVERSION= 0.1
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any mentions of this version string in the software's repo. It might be better to use g20250603 here - the date of the 8d28d3688accaa00cb5bf56919c57cfee134a937 commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

You also need to refresh distinfo after changing DISTVERSION.

do-build:
mkdir ${WRKSRC}/_build
cmake "${WRKSRC}/filedialogs/lunasvg" -B "${WRKSRC}/_build"
make -j${MAKE_JOBS_NUMBER} -C "${WRKSRC}/_build"
Copy link
Contributor

Choose a reason for hiding this comment

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

These steps seem to match with what USES=cmake does. Why have :indirect and put these commands into do-build?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me test that too.

@arrowd
Copy link
Contributor

arrowd commented Jun 3, 2025

It looks like your project statically links to various dependencies that wasn't developed by you. I'm not a lawyer at all, but are you sure that deps licenses permit that?

@samuelvenable
Copy link
Author

@arrowd My understanding on MIT is that as long as the license files remain in tact for all authors it should be fine. When i made this pull request I didn't really think about the fact the author listed in the port is just myself and doesn't include the other authors. i wasn't the one who wrote this port initially. I'll fix that now.

@arrowd
Copy link
Contributor

arrowd commented Jun 3, 2025

Is this software a plugin for either SDL or imgui?

@samuelvenable
Copy link
Author

im not sure, depends on how you define plugin.

@samuelvenable
Copy link
Author

samuelvenable commented Jun 3, 2025

@arrowd I updated the license in the root of the repository, it now includes mention of everyone involved that is statically linked afaik:
https://github.com/samuelvenable/SDL3-ImGui-FileDialogs/blob/main/LICENSE

I will add this license to be exported by the makefile

@samuelvenable
Copy link
Author

When this pr is approved, im too lazy to learn how to squash commits again; im going to close this pr and open a fresh one with the most up to date changes so it is all one commit.

@arrowd
Copy link
Contributor

arrowd commented Jun 4, 2025

A couple of points regarding this:

  • I'm still unsure what is this thing. How is it supposed to be used? If this is a library, are we going to have consumers for it in the ports tree?
  • The project still links all its deps statically. This goes against the flow in downstream package management. For instance, we do have x11-toolkits/imgui, graphics/lunasvg and devel/sdl3 in ports, so I'd expect this project to utilize some proper buildsystem to find these deps in the system prefix rather than bundling them.
  • A PR is basically a tuple of (your_branch, target_branch), so there is 0 sense to close PR and open another one. You can just force-push to the same your_branch and the PR will get updated.

@diizzyy
Copy link
Contributor

diizzyy commented Jun 4, 2025

The post-build section is a no go, fix Makefile or use something that works as intended

@samuelvenable
Copy link
Author

samuelvenable commented Jun 4, 2025

A couple of points regarding this:

* I'm still unsure what is this thing. How is it supposed to be used? If this is a library, are we going to have consumers for it in the ports tree?

* The project still links all its deps statically. This goes against the flow in downstream package management. For instance, we do have `x11-toolkits/imgui`, `graphics/lunasvg` and `devel/sdl3` in ports, so I'd expect this project to utilize some proper buildsystem to find these deps in the system prefix rather than bundling them.

* A PR is basically a tuple of `(your_branch, target_branch)`, so there is 0 sense to close PR and open another one. You can just force-push to the same `your_branch` and the PR will get updated.
  • If you are unsure what this is, I would recommend looking up zenity or kdialog. It's basically the same thing as those but instead of using GTK+ or Qt for the primary toolkit, it uses SDL and imgui. It is a way to create various dialog boxes (most notably file and directory selection) from shell scripts.

  • Why I'm currently statically linking imgui in particular is because the maintainers of imgui are known to publish API breaking changes without warning. Having my own version hosted with the repository limits API breakages and how often I would need to update the port. I didn't know lunasvg was an existing port. That's helpful to know, but it still raises similar concerns as imgui in terms of API breakage. I've added mention of the copyright holders to the LICENSE file, as you suggested, so I thought at first this would be a non-issue.

@samuelvenable
Copy link
Author

The post-build section is a no go, fix Makefile or use something that works as intended

Could you please be more specific on what the direct issue is? I have no idea what you are referring to. The Makefile used to be written wrongly in a way the produced executable would segfault, because I accidentally passed the -shared flag to it, however, this has since then been corrected and it both builds and works as intended from my testing now.

@diizzyy
Copy link
Contributor

diizzyy commented Jun 5, 2025

It's simply not maintainable, there are very few cases where we provide our own build / build file but that's in cases where there's an issue with upstream (dead, not accepting patches, unable to contact etc). Just fix it since you're upstream.

@samuelvenable
Copy link
Author

samuelvenable commented Jun 5, 2025

@diizzyy I don't mean to be annoying, but I still don't understand what you are trying to say. Fix what? Everything is working for me.

Edit:

Are you saying I shouldn't include build instructions in the Makefile? how is it supposed to build without it? Are you saying I should bulid using the Makefile in my repository and somehow call that Makefile from the port's Makeifle?

@diizzyy
Copy link
Contributor

diizzyy commented Jun 5, 2025

What I'm saying is that post-build shouldn't be needed at all, this should all be done in the project's Makefile (whatever it uses).

@kenrap
Copy link
Contributor

kenrap commented Jun 5, 2025

@diizzyy He is the upstream. His project uses build.sh build scripts, which didn't work well when I was writing the port for him. He would need to change his project to use Makefiles for building.

@samuelvenable
Copy link
Author

What I'm saying is that post-build shouldn't be needed at all, this should all be done in the project's Makefile (whatever it uses).

That's what I figured you meant, or at least something similar. Thanks for the info.

@samuelvenable
Copy link
Author

samuelvenable commented Jun 11, 2025

It looks like your project statically links to various dependencies that wasn't developed by you. I'm not a lawyer at all, but are you sure that deps licenses permit that?

I'm not sure how to most appropriately include the individual licenses and associated copyright holders. Should I have all the licenses and copyright holders in one file like I did before? Or should I keep them in different files, even if some of those files are the exact same license, just associated with a different project and/or copyright holder? I'm very new to this so I have no idea what is considered OK and what isn't. Should I give a title to each license file explaining which portions of the software the copyright and license covers? For example, for the imgui license, caption it with "imgui"?

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.

4 participants