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

Bugfixes, autotools update #1

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

tarjanm-movidius
Copy link

  • Updated autotools
  • Fixed all warnings to C++/17 standard
  • Fixed memory leak when reading std::map by element
  • Fixed NULL pointer segfault
  • Fixed overlapping window raise issue
  • Updated docs, added man page

@mgumz
Copy link
Member

mgumz commented Apr 18, 2022

hi @tarjanm-movidius,

i've checked locally fbpager plus your patches ontop of it.

the whole build-system should be moved more towards what fluxbox is using: autogen.sh (see https://github.com/fluxbox/fluxbox/blob/master/autogen.sh) … which is then generating configure, Makefile.in and all the related tools. I would rather remove all which is generated by autogen.sh.

even with your patches I ran into to problems to build fbpager ontop of aarch64. I then used autogen.sh as the base and came until:

git status
HEAD detached at gh.tm/master
Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
	modified:   .gitignore
	deleted:    Makefile.in
	deleted:    aclocal.m4
	new file:   autogen.sh
	modified:   compile
	deleted:    config.guess
	deleted:    configure
	deleted:    depcomp
	deleted:    ltmain.sh
	deleted:    missing
	deleted:    src/FbTk/Makefile.in
	deleted:    src/Makefile.in

The rest of your changes look valid. If you want, rework the build-system stuff by even further reducing it "to the max" and then I ll merge the whole thing.

@tarjanm-movidius
Copy link
Author

Hi Mathias,
Thank you, I'm perfectly happy with your changes, feel free to merge it. Left the build-system there for convenience, but a well automated autogen should be just as easy to use.
Feel free to merge & thanks again

@mgumz
Copy link
Member

mgumz commented Apr 19, 2022

@tarjanm-movidius then i ll cherry-pick the relevant changes from your branch and skip the build-part.

can't tell you "when" though.

thanks anyway!

@mgumz
Copy link
Member

mgumz commented Oct 5, 2022

@tarjanm-movidius ah, cool. I'll try your branch on the weekend. The delay was not intentionally, I am sorry about it.

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.

2 participants