Add dynamic path resolution for Windows - #36
Conversation
Signed-off-by: Arthit Suriyawongkul <arthit@gmail.com>
Signed-off-by: Arthit Suriyawongkul <arthit@gmail.com>
Signed-off-by: Arthit Suriyawongkul <arthit@gmail.com>
|
Btw, not sure if it is ok to add I can create a separate file ( |
|
Note that #12 and #30 also solve the same issue that this PR is trying to solve. But this PR only focuses on path resolution, not an entirely new build process. Comparisons:
|
Co-Authored-By: Theppitak Karoonboonyanan <631551+thep@users.noreply.github.com> Signed-off-by: Arthit Suriyawongkul <arthit@gmail.com>
Signed-off-by: Arthit Suriyawongkul <arthit@gmail.com>
From reviewer's code suggestion Signed-off-by: Arthit Suriyawongkul <arthit@gmail.com>
Co-Authored-By: Theppitak Karoonboonyanan <631551+thep@users.noreply.github.com> Signed-off-by: Arthit Suriyawongkul <arthit@gmail.com>
Co-Authored-By: Theppitak Karoonboonyanan <631551+thep@users.noreply.github.com> Signed-off-by: Arthit Suriyawongkul <arthit@gmail.com>
Signed-off-by: Arthit Suriyawongkul <arthit@gmail.com>
|
Updated the install dir finding algorithm per reviewer suggestion, with some adaptation for wide-char path in Windows. (see the flow table in the updated PR description for general idea; Step 3-4 are heuristic) @thep the libthai.c is now crowded with functions for Windows, do you have concerns on that? Or should we move them to a location specifically for utility functions? |
Yes, it should be good to have the Windows-specific stuffs in a separate file. Meanwhile, the static pointer |
|
It seems one comment I made before has been lost. So, let me repeat it here: In /* Then, fall back to default DICT_DIR macro */
if (!dict_trie) {
#if defined (_WIN32) && !defined (__CYGWIN__)
{
/* ... */
}
if (!dict_trie)
dict_trie = trie_new_from_file (DICT_DIR "/" DICT_NAME ".tri");
#else
dict_trie = trie_new_from_file (DICT_DIR "/" DICT_NAME ".tri");
#endif
}can be rearranged like this: #if defined (_WIN32) && !defined (__CYGWIN__)
/* Try to find dict under the base dir used to install the DLL */
if (!dict_trie) {
/* ... */
}
#endif
/* Then, fall back to default DICT_DIR macro */
if (!dict_trie) {
dict_trie = trie_new_from_file (DICT_DIR "/" DICT_NAME ".tri");
}That is, the Windows-specific stuff becomes just another step in between. |
Signed-off-by: Arthit Suriyawongkul <arthit@gmail.com>
| endif | ||
|
|
||
| libthai_la_SOURCES = libthai.c | ||
| libthai_la_SOURCES = libthai.c utils/win-utils.c |
There was a problem hiding this comment.
It's not quite good to refer to a file from different directory in Makefile.am. For example, this makes it more complicated to check whether a file is included in the release tarball prepared with make dist.
Instead, we may need to create another internal library libutils.la under the utils/ subdir. That is:
- In
src/Makefile.am, add$(top_builddir)/src/utils/libutils.latolibthai_sublibs. Previously, theutils/subdir just provided a plain header. Now it does provide some binary code. - In
src/utils/Makefile.am, replace thenoinst_HEADERStarget with this (withwin-utils.hsplit out ofpriv-utils.h):
noinst_LTLIBRARIES = libutils.la
libutils_la_SOURCES = \
priv-utils.h \
win-utils.h \
win-utils.c \
$(NULL)Some source files that previously included priv-utils.h for the Windows stuff may need to include win-utils.h instead, of course.
Per suggestions in review Co-Authored-By: Theppitak Karoonboonyanan <631551+thep@users.noreply.github.com> Signed-off-by: Arthit Suriyawongkul <arthit@gmail.com>
|
Looks good. Merged. Thank you very much for your work! |
|
Thanks for the reviews too. |
Find .dll for Windows to fix #7 and fix #17
Add
win_inst_dirto handle path resolution for Windows.Uses standard Windows APIs (partly based on #12 by @fanc999 and from @thep 's suggestion in #36 (comment)):
GetModuleHandleExW: To get the DLL module handle.GetModuleFileNameW: To retrieve the DLL's installation path (handle Unicode characters)Tested with mingw-w64 (cross-compiled), macOS, Ubuntu.
--
Path resolution flow:
GetModuleHandleExW(inwin_inst_dir)GetModuleFileNameW(inwin_inst_dir)C:\App\bin\libthai.dllorC:\App2\libthai.dll, or NULL on failurewin_find_inst_dirC:\App(remove "bin") orC:\App2(no "bin" found, just use the full dir), orNULLon malloc failurewfull_path(inbrk_load_default_dict)C:\App\share\libthai\thbrk.triorC:\App2\share\libthai\thbrk.tri, or NULL on malloc failure_wfopen(inbrk_load_default_dict)trie_fread(inbrk_load_default_dict)Trie *dict_trie, or NULL if the file is badtrie_new_from_file(DICT_DIR "/" DICT_NAME ".tri")(inbrk_load_default_dict)