github-starred/libxlsxwriter
2
0
Fork 0
mirror of https://github.com/jmcnamara/libxlsxwriter.git synced 2026-05-15 14:15:54 -06:00
Code Issues 16 Projects Packages Wiki Activity Actions

[GH-ISSUE #265] Possible typos in makefiles #213

New issue
Closed
opened 2026-05-05 11:56:18 -06:00 by gitea-mirror · 15 comments
gitea-mirror commented 2026-05-05 11:56:18 -06:00
Owner
Copy link

Originally created by @utelle on GitHub (Jan 15, 2020).
Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/265

Originally assigned to: @jmcnamara on GitHub.

Today I tried to update my clone of libxlsxwriter to your latest master. While trying to track down why my travis CI fails for the test_valgrind cases (I get duplicate symbols on linking, and I would be grateful if could take a look why this might be so - TIA), I found a few possible typos in your makefiles:

  • line 14 in Makefile and src/Makefile defines symbol DESTIDR - shouldn't that be DESTDIR?
  • line 49 in Makefile in the clean target uses symbol USE_STANDARD_TMPFILE to clean the minizip files - shouldn't that be symbol USE_SYSTEM_MINIZIP?
Originally created by @utelle on GitHub (Jan 15, 2020). Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/265 Originally assigned to: @jmcnamara on GitHub. Today I tried to update my clone of libxlsxwriter to your latest master. While trying to track down why my travis CI fails for the [test_valgrind](https://travis-ci.org/utelle/libxlsxwriter/jobs/637393795?utm_medium=notification&utm_source=github_status) cases (_I get duplicate symbols on linking, and I would be grateful if could take a look why this might be so - TIA_), I found a few possible typos in your makefiles: - line 14 in `Makefile` and `src/Makefile` defines symbol `DESTIDR` - shouldn't that be `DESTDIR`? - line 49 in `Makefile` in the `clean` target uses symbol `USE_STANDARD_TMPFILE` to clean the minizip files - shouldn't that be symbol `USE_SYSTEM_MINIZIP`?
gitea-mirror 2026-05-05 11:56:18 -06:00
  • closed this issue
  • added the
    bug
         label
gitea-mirror commented 2026-05-05 11:56:27 -06:00
Author
Owner
Copy link

@jmcnamara commented on GitHub (Jan 15, 2020):

Hi,

Thanks those are both typos/bugs.

I had a quick look at your fork and I don't see anything that would cause that link issue. I'll look at it in more detail and let you know.

<!-- gh-comment-id:574657532 --> @jmcnamara commented on GitHub (Jan 15, 2020): Hi, Thanks those are both typos/bugs. I had a quick look at your fork and I don't see anything that would cause that link issue. I'll look at it in more detail and let you know.
gitea-mirror commented 2026-05-05 11:56:31 -06:00
Author
Owner
Copy link

@utelle commented on GitHub (Jan 15, 2020):

I had a quick look at your fork and I don't see anything that would cause that link issue. I'll look at it in more detail and let you know.

Great. Thanks. Most likely it is some small an subtle issue.

<!-- gh-comment-id:574660120 --> @utelle commented on GitHub (Jan 15, 2020): > I had a quick look at your fork and I don't see anything that would cause that link issue. I'll look at it in more detail and let you know. Great. Thanks. Most likely it is some small an subtle issue.
gitea-mirror commented 2026-05-05 11:56:34 -06:00
Author
Owner
Copy link

@jmcnamara commented on GitHub (Jan 15, 2020):

I ran the test against your branch/fork and I didn't see the issue. Maybe just resubmit/rerun the test by creating a new branch or pushing a minor change.

<!-- gh-comment-id:574686346 --> @jmcnamara commented on GitHub (Jan 15, 2020): I ran the test against your branch/fork and I didn't see the issue. Maybe just resubmit/rerun the test by creating a new branch or pushing a minor change.
gitea-mirror commented 2026-05-05 11:56:38 -06:00
Author
Owner
Copy link

@jmcnamara commented on GitHub (Jan 15, 2020):

@sjmulder on the DESTIDR typo:

https://github.com/jmcnamara/libxlsxwriter/blob/master/Makefile#L14

<!-- gh-comment-id:574696104 --> @jmcnamara commented on GitHub (Jan 15, 2020): @sjmulder on the DESTIDR typo: https://github.com/jmcnamara/libxlsxwriter/blob/master/Makefile#L14
gitea-mirror commented 2026-05-05 11:56:42 -06:00
Author
Owner
Copy link

@sjmulder commented on GitHub (Jan 15, 2020):

@jmcnamara yes that's a typo - I can fix it or someone else can. But it doesn't break anything as-as since the line is a no-op intended for documentation (or easy tweaking).

<!-- gh-comment-id:574699700 --> @sjmulder commented on GitHub (Jan 15, 2020): @jmcnamara yes that's a typo - I can fix it or someone else can. But it doesn't break anything as-as since the line is a no-op intended for documentation (or easy tweaking).
gitea-mirror commented 2026-05-05 11:56:45 -06:00
Author
Owner
Copy link

@utelle commented on GitHub (Jan 15, 2020):

I ran the test against your branch/fork and I didn't see the issue. Maybe just resubmit/rerun the test by creating a new branch or pushing a minor change.

I restarted the failing job, but it failed again. Only the test_valgrind part fails, that is:

env:  - CFLAGS='-Werror'
script:  - make test_valgrind USE_DOUBLE_FUNCTION=1 V=1

I'm at a loss why the link step fails with duplicate symbols.

<!-- gh-comment-id:574699851 --> @utelle commented on GitHub (Jan 15, 2020): > I ran the test against your branch/fork and I didn't see the issue. Maybe just resubmit/rerun the test by creating a new branch or pushing a minor change. I restarted the failing job, but it failed again. Only the `test_valgrind` part fails, that is: ``` env: - CFLAGS='-Werror' script: - make test_valgrind USE_DOUBLE_FUNCTION=1 V=1 ``` I'm at a loss why the link step fails with duplicate symbols.
gitea-mirror commented 2026-05-05 11:56:46 -06:00
Author
Owner
Copy link

@utelle commented on GitHub (Jan 15, 2020):

As recommended in the error message I added the linker option -v (see verbose test_valgrind). This revealed that the object file for utility.c is contained 2 times in the library file, as utility.o and as utility.c.o. This seems to be a side effect of the test_cmake script. Not sure what's going on here.

<!-- gh-comment-id:574861194 --> @utelle commented on GitHub (Jan 15, 2020): As recommended in the error message I added the linker option `-v` (see [verbose test_valgrind](https://travis-ci.org/utelle/libxlsxwriter/jobs/637633297?utm_medium=notification&utm_source=github_status)). This revealed that the object file for `utility.c` is contained 2 times in the library file, as `utility.o` and as `utility.c.o`. This seems to be a side effect of the `test_cmake` script. Not sure what's going on here.
gitea-mirror commented 2026-05-05 11:56:49 -06:00
Author
Owner
Copy link

@utelle commented on GitHub (Jan 15, 2020):

As an attempt of a workaround I moved the test_cmake to the end of the script list. This helped a bit, but the second test_valgrind run still fails (see travis CI).

<!-- gh-comment-id:574876417 --> @utelle commented on GitHub (Jan 15, 2020): As an attempt of a workaround I moved the `test_cmake` to the end of the script list. This helped a bit, but the second `test_valgrind` run still fails (see [travis CI](https://travis-ci.org/utelle/libxlsxwriter?utm_medium=notification&utm_source=github_status)).
gitea-mirror commented 2026-05-05 11:56:53 -06:00
Author
Owner
Copy link

@jmcnamara commented on GitHub (Jan 16, 2020):

I was able to reproduce this. There is a memory leak running under gcc. It is a little odd but at least I can reproduce it. I'm working on a fix.

<!-- gh-comment-id:574920293 --> @jmcnamara commented on GitHub (Jan 16, 2020): I was able to reproduce this. There is a memory leak running under gcc. It is a little odd but at least I can reproduce it. I'm working on a fix.
gitea-mirror commented 2026-05-05 11:56:55 -06:00
Author
Owner
Copy link

@utelle commented on GitHub (Jan 16, 2020):

Thanks for looking into this.

Moving the test_cmake to the end of the script list is in fact only a workaround. At least in my branch the problem arises from cmake using extension *.c.o for object files instead of just *.o and adding those files to the existing library file. AFAICT this behaviour is also shown in your repository. Strange enough it does not result in errors for your CI runs.

<!-- gh-comment-id:575108505 --> @utelle commented on GitHub (Jan 16, 2020): Thanks for looking into this. Moving the `test_cmake` to the end of the script list is in fact only a workaround. At least in my branch the problem arises from cmake using extension `*.c.o` for object files instead of just `*.o` and adding those files to the existing library file. AFAICT this behaviour is also shown in your repository. Strange enough it does not result in errors for your CI runs.
gitea-mirror commented 2026-05-05 11:56:57 -06:00
Author
Owner
Copy link

@jmcnamara commented on GitHub (Jan 16, 2020):

There are 2 issues.

  1. The CMake issue, that I don't understand but will look into. Moving it to the end or adding a "make clean" after it runs should be okay.
  2. There is a valgrind warning that only occurs for some gcc/valgrind combination.

There is a fix for the valgrind warning here: 335548e77b

However, I haven't bottomed out on whether this is a valid warning/error/fix. I'm still looking into it.

<!-- gh-comment-id:575112621 --> @jmcnamara commented on GitHub (Jan 16, 2020): There are 2 issues. 1. The CMake issue, that I don't understand but will look into. Moving it to the end or adding a "make clean" after it runs should be okay. 2. There is a valgrind warning that only occurs for some gcc/valgrind combination. There is a fix for the valgrind warning here: https://github.com/jmcnamara/libxlsxwriter/commit/335548e77b8425a6fa3611315ab9ba8e3c4da0cb However, I haven't bottomed out on whether this is a valid warning/error/fix. I'm still looking into it.
gitea-mirror commented 2026-05-05 11:56:59 -06:00
Author
Owner
Copy link

@jmcnamara commented on GitHub (Jan 19, 2020):

I've fixed the typos and valgrind issue on master. Thanks for the report.

<!-- gh-comment-id:576026164 --> @jmcnamara commented on GitHub (Jan 19, 2020): I've fixed the typos and valgrind issue on master. Thanks for the report.
gitea-mirror commented 2026-05-05 11:57:03 -06:00
Author
Owner
Copy link

@utelle commented on GitHub (Jan 20, 2020):

Thanks again for your excellent support.

With your modifications my library variant now passes again all CI tests.

BTW, maybe you should consider to edit the release titles of your release 0.9.2 to 0.9.4, since they reference January 2019 instead of January 2020.

<!-- gh-comment-id:576244215 --> @utelle commented on GitHub (Jan 20, 2020): Thanks again for your excellent support. With your modifications my library variant now passes again all CI tests. BTW, maybe you should consider to edit the release titles of your release 0.9.2 to 0.9.4, since they reference January 2019 instead of January 2020.
gitea-mirror commented 2026-05-05 11:57:05 -06:00
Author
Owner
Copy link

@jmcnamara commented on GitHub (Jan 20, 2020):

BTW, maybe you should consider to edit the release titles of your release 0.9.2 to 0.9.4, since they reference January 2019 instead of January 2020.

Lol. Thanks.

Btw, I've started to look at adding a third party sprintf lib to fix #64. From a code point of view my preference is for fpconv instead of emyg_dtoa but I'll test both.

<!-- gh-comment-id:576248157 --> @jmcnamara commented on GitHub (Jan 20, 2020): > BTW, maybe you should consider to edit the release titles of your release 0.9.2 to 0.9.4, since they reference January 2019 instead of January 2020. Lol. Thanks. Btw, I've started to look at adding a third party sprintf lib to fix #64. From a code point of view my preference is for fpconv instead of emyg_dtoa but I'll test both.
gitea-mirror commented 2026-05-05 11:57:08 -06:00
Author
Owner
Copy link

@utelle commented on GitHub (Jan 20, 2020):

I will take a look at fpconv again. As mentioned in my post of August 10, 2017 fpconv is based on the same algorithm as emyg_dtoa. So in principle it should work equally well (although according to the benchmark it is slower by a factor of about 2).

Nevertheless, my adjusted version of emyg_dtoa obviously seems to work properly, otherwise my library variant wouldn't pass all the extensive CI tests of libxlsxwriter.

<!-- gh-comment-id:576261353 --> @utelle commented on GitHub (Jan 20, 2020): I will take a look at `fpconv` again. As mentioned in my [post of August 10, 2017](https://github.com/jmcnamara/libxlsxwriter/issues/64#issuecomment-321398715) `fpconv` is based on the same algorithm as `emyg_dtoa`. So in principle it should work equally well (although according to the benchmark it is slower by a factor of about 2). Nevertheless, my adjusted version of `emyg_dtoa` obviously seems to work properly, otherwise my library variant wouldn't pass all the extensive CI tests of libxlsxwriter.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
coverity
cmake-improvements
cmake_pkgconfig_fix
ctest
button
tables
autofilter
dtoa
iwyu
openssl
gentoo
gh310
condition_format
rich_string_html
header_footer_images
custom_data_labels
object_position
gh265
constant_mode_comments2
constant_mode_comments
vml
md5
vcpkg
hyperlinks
api_refactor
win_fopen
gh203
rich_strings
chartsheet
in_mem_image
gh192
revert-162-master
data_validation
meson
charts
scan_build
utf8_strlen
tmpfileplus
custom_properties
insert_image
default_row
control_chars
worksheet_protect
dev
red_black
panes
mkstemp
defined_names
optimise
wip
v1.2.4
v1.2.3
v1.2.2
v1.2.1
v1.2.0
v1.1.9
v1.1.8
RELEASE_1.1.7
RELEASE_1.1.6
RELEASE_1.1.5
RELEASE_1.1.4
RELEASE_1.1.3
RELEASE_1.1.2
RELEASE_1.1.1
RELEASE_1.1.0
RELEASE_1.0.9
RELEASE_1.0.8
RELEASE_1.0.7
RELEASE_1.0.6
RELEASE_1.0.5
RELEASE_1.0.4
RELEASE_1.0.3
RELEASE_1.0.2
RELEASE_1.0.1
RELEASE_1.0.0
RELEASE_0.9.9
RELEASE_0.9.8
RELEASE_0.9.7
RELEASE_0.9.6
RELEASE_0.9.5
RELEASE_0.9.4
RELEASE_0.9.3
RELEASE_0.9.2
RELEASE_0.9.1
RELEASE_0.9.0
RELEASE_0.8.9
RELEASE_0.8.8
RELEASE_0.8.7
RELEASE_0.8.6
RELEASE_0.8.5
RELEASE_0.8.4
RELEASE_0.8.3
RELEASE_0.8.2
RELEASE_0.8.1
RELEASE_0.8.0
RELEASE_0.7.9
RELEASE_0.7.8
RELEASE_0.7.7
RELEASE_0.7.6
RELEASE_0.7.5
RELEASE_0.7.4
RELEASE_0.7.3
RELEASE_0.7.2
RELEASE_0.7.1
RELEASE_0.7.0
RELEASE_0.6.9
RELEASE_0.6.8
RELEASE_0.6.7
RELEASE_0.6.6
RELEASE_0.6.5
RELEASE_0.6.4
RELEASE_0.6.3
RELEASE_0.6.2
RELEASE_0.6.1
RELEASE_0.6.0
RELEASE_0.5.9
RELEASE_0.5.8
RELEASE_0.5.7
RELEASE_0.5.6
RELEASE_0.5.5
RELEASE_0.5.4
RELEASE_0.5.3
RELEASE_0.5.2
RELEASE_0.5.1
RELEASE_0.5.0
RELEASE_0.4.9
RELEASE_0.4.8
RELEASE_0.4.7
RELEASE_0.4.6
RELEASE_0.4.5
RELEASE_0.4.4
RELEASE_0.4.3
RELEASE_0.4.2
RELEASE_0.4.1
RELEASE_0.4.0
RELEASE_0.3.9
RELEASE_0.3.8
RELEASE_0.3.7
RELEASE_0.3.6
RELEASE_0.3.5
RELEASE_0.3.4
RELEASE_0.3.3
RELEASE_0.3.2
RELEASE_0.3.1
RELEASE_0.3.0
RELEASE_0.2.9
RELEASE_0.2.8
RELEASE_0.2.7
RELEASE_0.2.6
RELEASE_0.2.5
RELEASE_0.2.4
RELEASE_0.2.3
RELEASE_0.2.2
RELEASE_0.2.1
RELEASE_0.2.0
RELEASE_0.1.9
RELEASE_0.1.8
RELEASE_0.1.7
RELEASE_0.1.6
RELEASE_0.1.5
RELEASE_0.1.4
RELEASE_0.1.3
RELEASE_0.1.2
RELEASE_0.1.1
RELEASE_0.1.0
RELEASE_0.0.9
RELEASE_0.0.8
RELEASE_0.0.7
RELEASE_0.0.6
RELEASE_0.0.5
RELEASE_0.0.4
RELEASE_0.0.3
RELEASE_0.0.2
RELEASE_0.0.1
Labels
Clear labels   
awaiting user feedback

   
bug

   
cmake

   
cmake

   
docs

   
feature request

   
in progress

   
long term

   
medium term

   
medium term

   
pull-request

Mirrored from GitHub Pull Request

  
question

   
question

   
ready to close

   
short term

   
under investigation

   
wontfix
No labels
awaiting user feedback
bug
cmake
cmake
docs
feature request
in progress
long term
medium term
medium term
pull-request
question
question
ready to close
short term
under investigation
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/libxlsxwriter#213
No description provided.
Delete branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?