Change large file support to more portable #2

Злито
SlavekB злито 1 комітів з feat/portable-large-file-support до master 5 роки тому
obache прокоментував(ла) 5 роки тому
Співавтор

I believe lseek64/off64_t is only supported for some platforms (glibc and solaris ?).

KDE_CHECK_LARGEFILE will set sufficient compiler flags to enable 64bit capable lseek/off_t,
so it is portable to use the autoconf macro and lseek/off_t instead of lseek64/off64_t with
-D_LARGEFILE64_SOURCE.

I believe lseek64/off64_t is only supported for some platforms (glibc and solaris ?). KDE_CHECK_LARGEFILE will set sufficient compiler flags to enable 64bit capable lseek/off_t, so it is portable to use the autoconf macro and lseek/off_t instead of lseek64/off64_t with -D_LARGEFILE64_SOURCE.
SlavekB прокоментував(ла) 5 роки тому
Власник

Please correct the commit log – WIP probably should not be part of the commit log and there is a typo chnage × change.

Please correct the commit log – WIP probably should not be part of the commit log and there is a typo chnage × change.
Ghost прокоментував(ла) 5 роки тому

Talking about portability, shouldn't "-D_GNU_SOURCE" be changed for "-D_DEFAULT_SOURCE" in kttsd/players/alsaplayer/Makefile.am?

Talking about portability, shouldn't "-D_GNU_SOURCE" be changed for "-D_DEFAULT_SOURCE" in kttsd/players/alsaplayer/Makefile.am?
SlavekB прокоментував(ла) 5 роки тому
Власник

Replacing _BSD_SOURCE, _SVID_SOURCE, _GNU_SOURCE with _DEFAULT_SOURCE has already been done by Michele some time ago. Some occurrences appear to have been omitted. I leave this for @MicheleC to check.

Replacing _BSD_SOURCE, _SVID_SOURCE, _GNU_SOURCE with _DEFAULT_SOURCE has already been done by Michele some time ago. Some occurrences appear to have been omitted. I leave this for @MicheleC to check.
MicheleC прокоментував(ла) 5 роки тому
Власник

_GNU_SOURCE was not replaced. During the replacement of _BSD_SOURCE and _SVID_SOURCE, I also tested replacing _GNU_SOURCE with _DEFAULT_SOURCE but this led to compile errors since _GNU_SOURCE makes GNU extensions available.
_GNU_SOURCE still needs to be used.

_GNU_SOURCE was not replaced. During the replacement of _BSD_SOURCE and _SVID_SOURCE, I also tested replacing _GNU_SOURCE with _DEFAULT_SOURCE but this led to compile errors since _GNU_SOURCE makes GNU extensions available. _GNU_SOURCE still needs to be used.
SlavekB прокоментував(ла) 5 роки тому
Власник

I tested build on Debian 10 (Buster) amd64, i386 and armhf – all successful.

There is a question: When building with CMake, there is no equivalent test for large files support. In some CMakeLists.txt we can see hard-coded _LARGEFILE64_SOURCE=1 – for example tdelibs/tdeio/CMakeLists.txt, but there is no test if some of the definitions like _LARGE_FILES, _LARGEFILE_SOURCE and _FILE_OFFSET_BITS=64 are needed. Should we add an equivalent test for large files to a common CMake module?

I tested build on Debian 10 (Buster) amd64, i386 and armhf – all successful. There is a question: When building with CMake, there is no equivalent test for large files support. In some CMakeLists.txt we can see hard-coded `_LARGEFILE64_SOURCE=1` – for example [tdelibs/tdeio/CMakeLists.txt](../tdelibs/src/branch/master/tdeio/CMakeLists.txt#L13), but there is no test if some of the definitions like `_LARGE_FILES`, `_LARGEFILE_SOURCE` and `_FILE_OFFSET_BITS=64` are needed. Should we add an equivalent test for large files to a common CMake module?
MicheleC прокоментував(ла) 5 роки тому
Власник

Should we add an equivalent test for large files to a common CMake module?

Sounds like a good idea.

```Should we add an equivalent test for large files to a common CMake module?``` Sounds like a good idea.
MicheleC прокоментував(ла) 5 роки тому
Власник

@obache is it ok to merge? or is this PR marked WIP for a reason (e.g. are you planning more work on this)?

BTW, off64_t is also used in other parts of TDE. It could be a good idea to make similar changes to increase portability.

@obache is it ok to merge? or is this PR marked WIP for a reason (e.g. are you planning more work on this)? BTW, off64_t is also used in other parts of TDE. It could be a good idea to make similar changes to increase portability.
obache прокоментував(ла) 5 роки тому
Автор
Співавтор

Adding "WIP" is just following the guide.

Adding "WIP" is just following the guide.
SlavekB прокоментував(ла) 5 роки тому
Власник

We use the WIP label to indicate that the author of PR will still be working on this. If the author thinks, that PR is ready, then will delete the WIP label. That's why it is our question, whether from your perspective PR is still WIP or ready.

We use the WIP label to indicate that the author of PR will still be working on this. If the author thinks, that PR is ready, then will delete the WIP label. That's why it is our question, whether from your perspective PR is still WIP or ready.
obache прокоментував(ла) 5 роки тому
Автор
Співавтор

You can remove WIP label if this PR is acceptable for you.

I don't know whether it is requested to replace _GNU_SOURCE and convert to cmake additionally.

You can remove WIP label if this PR is acceptable for you. I don't know whether it is requested to replace _GNU_SOURCE and convert to cmake additionally.
SlavekB змінився заголовок з WIP: change large file support to more portable на Change large file support to more portable 5 роки тому
SlavekB закрив цей запит на злиття 5 роки тому
SlavekB видалена гілка feat/portable-large-file-support 5 роки тому
SlavekB додав(ла) до R14.0.6 release етапу 5 роки тому
Запит на злиття був влитиий як 12ef306abc.
Підпишіться щоб приєднатися до обговорення.
Немає рецензентів
Етап відсутній
Немає виконавця
4 учасників
Сповіщення
Дата завершення

Термін виконання не встановлений.

Залежності

No dependencies set.

Reference: TDE/tdeaccessibility#2
Завантаження…
Тут ще немає жодного змісту.