public inbox for bzip2-devel@sourceware.org
 help / color / mirror / Atom feed
* Re: _stati64 patch (Was: [PATCH] Fix include path separator)
  2019-01-01  0:00     ` _stati64 patch (Was: [PATCH] Fix include path separator) Mark Wielaard
@ 2019-01-01  0:00       ` Joshua Watt
  2019-01-01  0:00         ` Mark Wielaard
  0 siblings, 1 reply; 11+ messages in thread
From: Joshua Watt @ 2019-01-01  0:00 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: bzip2-devel, Phil Ross


On 7/5/19 3:38 AM, Mark Wielaard wrote:
> Hi,
>
> On Wed, Jul 03, 2019 at 03:49:45PM -0500, Joshua Watt wrote:
>> I did tests with msvc 18.0 (visual studio 2013), and I can probably get
>> some newer versions if you want. I can also try with mingw gcc (on Windows;
>> already verified on Linux) if you like... I'm not sure what else would be a
>> relevant test
> There is another Windows specific patch which we haven't applied yet
> to the 1.0.x branch because we don't have Windows builders/testers.
>
> It is the attached patch from Phil Ross (CCed) to use _stati64 instead
> of _stat to support large >4GB files on Windows.
>
> If you could test the above works on your setups that would be
> appreciated.  Do we need any checks to see whether _stati64 is
> available? Or can it be used unconditionally as in this patch?

The patch compiles without issues using msvc 18.0 (Visual Studio 2013) 
and 64-bit MinGW gcc 7.3.0. 'make test' passes also.

The "patch in a patch" format was a little confusing at first :)

>
> Thanks,
>
> Mark

^ permalink raw reply	[flat|nested] 11+ messages in thread

* _stati64 patch (Was: [PATCH] Fix include path separator)
       [not found]   ` <CAJdd5GZfTk8EKeUE_C1keKm+gn=qapv9OQGGariiiXP9jyu0wQ@mail.gmail.com>
  2019-01-01  0:00     ` Mark Wielaard
@ 2019-01-01  0:00     ` Mark Wielaard
  2019-01-01  0:00       ` Joshua Watt
  1 sibling, 1 reply; 11+ messages in thread
From: Mark Wielaard @ 2019-01-01  0:00 UTC (permalink / raw)
  To: Joshua Watt; +Cc: bzip2-devel, Phil Ross

[-- Attachment #1: Type: text/plain, Size: 775 bytes --]

Hi,

On Wed, Jul 03, 2019 at 03:49:45PM -0500, Joshua Watt wrote:
> I did tests with msvc 18.0 (visual studio 2013), and I can probably get
> some newer versions if you want. I can also try with mingw gcc (on Windows;
> already verified on Linux) if you like... I'm not sure what else would be a
> relevant test

There is another Windows specific patch which we haven't applied yet
to the 1.0.x branch because we don't have Windows builders/testers.

It is the attached patch from Phil Ross (CCed) to use _stati64 instead
of _stat to support large >4GB files on Windows.

If you could test the above works on your setups that would be
appreciated.  Do we need any checks to see whether _stati64 is
available? Or can it be used unconditionally as in this patch?

Thanks,

Mark

[-- Attachment #2: 0001-Fix-a-not-a-normal-file-error-when-compressing-large.patch --]
[-- Type: text/x-diff, Size: 1337 bytes --]

From c0ee33ad3d6108eb6f7ddaf71bcc22ea7a3f855f Mon Sep 17 00:00:00 2001
From: Phil Ross <phil.ross@gmail.com>
Date: Tue, 21 May 2019 20:46:14 +0100
Subject: [PATCH] Fix a 'not a normal file' error when compressing large files.

The bzip2 command line would report 'not a normal file' for files of
size larger than 2^32 - 1 bytes.

Patch bzip2.c to use _stati64 instead of _stat so that a successful
result is returned for large files.

Resolves https://github.com/philr/bzip2-windows/issues/3.
---
 patches/06-support_64bit_file_sizes.diff | 13 +++++++++++++
 1 file changed, 13 insertions(+)
 create mode 100644 patches/06-support_64bit_file_sizes.diff

diff --git a/patches/06-support_64bit_file_sizes.diff b/patches/06-support_64bit_file_sizes.diff
new file mode 100644
index 0000000..abf1425
--- /dev/null
+++ b/patches/06-support_64bit_file_sizes.diff
@@ -0,0 +1,13 @@
+--- bzip2-1.0.6.orig/bzip2.c	2010-09-11 00:04:53.000000000 +0100
++++ bzip2-1.0.6/bzip2.c	2019-05-21 20:40:43.699892600 +0100
+@@ -132,8 +132,8 @@
+ 
+ #   define NORETURN       /**/
+ #   define PATH_SEP       '\\'
+-#   define MY_LSTAT       _stat
+-#   define MY_STAT        _stat
++#   define MY_LSTAT       _stati64
++#   define MY_STAT        _stati64
+ #   define MY_S_ISREG(x)  ((x) & _S_IFREG)
+ #   define MY_S_ISDIR(x)  ((x) & _S_IFDIR)
+ 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: _stati64 patch (Was: [PATCH] Fix include path separator)
  2019-01-01  0:00                 ` Mark Wielaard
@ 2019-01-01  0:00                   ` Joshua Watt
  0 siblings, 0 replies; 11+ messages in thread
From: Joshua Watt @ 2019-01-01  0:00 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: bzip2-devel, Phil Ross

On Mon, Jul 15, 2019 at 5:27 PM Mark Wielaard <mark@klomp.org> wrote:
>
> Hi Joshua,
>
> On Mon, 2019-07-15 at 08:58 -0500, Joshua Watt wrote:
> > On 7/14/19 4:23 PM, Mark Wielaard wrote:
> > >
> > > That is really awesome. Thanks so much for testing that out.
> > > Can a cross build bzip2 using MinGw (and msys) be run under Wine?
> > > That might give us a build CI pipeline for testing bzip2 using the
> > > buildbot. It might not be identical to running under actual Windows.
> > > But it might be scripted/automated.
> >
> > We do something similar to that to test our MinGW SDKs for the Yocto
> > Project. We cross compile the SDK using the MinGW toolchain on Linux
> > (note that MSYS is not required; the Linux system already has all the
> > POSIX tools), then run a set of automated tests under Wine.
>
> OK nice. So for testing you can just use the normal scripts/tools, just
> make sure the binary itself is ran through wine?

correct

>
> > You might be able to take this approach also. It wouldn't verify that
> > you can actually compile under Windows (as stated, it is a cross compile
> > from Linux), but it would give some amount of confidence that you can
> > actually run the cross compiled bzip2 on Windows and it will pass the
> > tests. You are correct that Wine isn't a fully faithful reproduction of
> > Windows; I've found a few things that just refuse to run properly under
> > wine, but do fine on actual Windows.
>
> Right. We only want to test that it builds as if for Windows. That is
> test the code with BZ_UNIX 0 and BZ_LCCWIN32 1 compiles correctly.
>
> > Most distros have a ready-made MinGW GCC compiler that can be installed.
> > The harder part is getting any additional dependencies. Most of the
> > distros I've seen don't have very many of the MinGW cross compiled
> > library (why would they?) so if your trying to cross compile a large
> > codebase, you can easily get into trouble with missing dependencies. The
> > Yocto project has an advantage in this regard because we are already
> > compiling everything from source anyway, so cross compiling the
> > requisite dependencies for MinGW isn't any additional work. I think that
> > bzip2 might also be able to be cross compiled easily because it doesn't
> > have very many dependencies.
>
> As far as I know bzip2 really shouldn't have any dependencies except
> for what is provided by MinGW GCC itself.
>
> > If you want to verify that you can compile under Windows, you might be
> > able to install MSVC in wine and use it to build bzip2. You *might* also
> > be able to install the Windows version of MinGW and MSYS in wine. I
> > don't know how stable this would be or if it would work at all.
>
> Both may go a bit too far. Just being able to (cross) compile to a
> Windows binary and then check it works as expected seems all that is
> really necessary to make sure we don't break thing (too much).
>
> Lets see... So I installed mingw32-gcc and wine for my distro.
>
> $ make CC=i686-w64-mingw32-gcc
> [... mostly seems to work ...]
> i686-w64-mingw32-gcc -Wall -Winline -O2 -g -D_FILE_OFFSET_BITS=64  -o
> bzip2 bzip2.o -L. -lbz2
> ./libbz2.a: error adding symbols: Archive has no index; run ranlib to
> add one
> collect2: error: ld returned 1 exit status
> make: *** [bzip2] Error 1
>
> OK, that is odd, but lets do what it says:
> $ i686-w64-mingw32-ranlib ./libbz2.a
>
> $ i686-w64-mingw32-gcc -Wall -Winline -O2 -g -D_FILE_OFFSET_BITS=64 \
>                        -o bzip2 bzip2.o -L. -lbz2
>
> $ file bzip2
> ./bzip2: PE32 executable (console) Intel 80386, for MS Windows
>
> Nice!
>
> But...
> $ wine ./bzip2
> wine: Bad EXE format for Z:\home\mark\src\bzip2-work\bzip2..
>
> OK, apparently I needed mingw64-gcc.
>
> So, same as above with make CC=i686-w64-mingw32-gcc
> x86_64-w64-mingw32-ranlib ./libbz2.a and
> x86_64-w64-mingw32-gcc -Wall -Winline -O2 -g -D_FILE_OFFSET_BITS=64  -o
> bzip2 bzip2.o -L. -lbz2

I think some of the distro support for MinGW is a little quirky.

I ran into these same issues with ranlib and the executable not
running. It's been a while, but I *think* that you can compile for
32-bit windows by always using the 64-bit compiler, but telling it to
produce 32-bit executables, i.e.:

 CC="x86_64-w64-mingw32-gcc -m32"

I never was able to dig up an actual 32-bit install of Windows to test
that it would actually run there though. I also suspect that only
supporting 64-bit would be acceptable at this point, so it may not
matter too much.

>
> $ file bzip2
> bzip2: PE32+ executable (console) x86-64, for MS Windows
>
> $ wine ./bzip2 --help
> bzip2, a block-sorting file compressor.  Version 1.0.8, 13-Jul-2019.
>
>    usage: bzip2 [flags and input files in any order]
>
>    -h --help           print this message
>    -d --decompress     force decompression
>    -z --compress       force compression
>    -k --keep           keep (don't delete) input files
>    -f --force          overwrite existing output files
>    -t --test           test compressed file integrity
>    -c --stdout         output to standard out
>    -q --quiet          suppress noncritical error messages
>    -v --verbose        be verbose (a 2nd -v gives more)
>    -L --license        display software version & license
>    -V --version        display software version & license
>    -s --small          use less memory (at most 2500k)
>    -1 .. -9            set block size to 100k .. 900k
>    --fast              alias for -1
>    --best              alias for -9
>
>    If invoked as `bzip2', default action is to compress.
>               as `bunzip2',  default action is to decompress.
>               as `bzcat', default action is to decompress to stdout.
>
>    If no file names are given, bzip2 compresses or decompresses
>    from standard input to standard output.  You can combine
>    short flags, so `-v -4' means the same as -v4 or -4v, &c.
>
>
> That is so awesome! Even though I am not exactly sure what I did.
> But it seems possible with some tweaks to get this working.
>
> Could you share some hints and tips on your setup? Or simply explain
> what went wrong in the above or what accidentally worked and why?
>
> Also is there a "command line wine"?

Not really. Much like Windows proper, wine includes the whole GUI
library. The only way to not show any of the GUI is to be careful not
to invoke any programs that might show it.

> The first time you run wine it seems to spawn all kinds of windows
> setup thingies. That would work on the buildbots of course.

You can still do everything that you care about on the buildbots,
since bzip2 doesn't actually do anything with the GUI, it won't be
displayed.
There are two environment variables you want to set, WINEPREFIX and
WINEARCH. See https://linuxconfig.org/using-wine-prefixes .

In your CI tests you will want to do something like this:

 export WINEPREFIX=$(mktemp -d) # Make a new empty wine installation
(a.k.a bottle)
 export WINEARCH=win64 # Make the new bottle a 64-bit install... can
also be win32 for a 32-bit install.
 wine ./bzip2 # Run bzip2 in the new bottle

This ensures you get a fresh wine install with the default
configuration each time the CI job runs (of course, you may want to do
something other than mktemp -d for your temp dir).

>
> Thanks,
>
> Mark

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fix include path separator
       [not found]   ` <CAJdd5GZfTk8EKeUE_C1keKm+gn=qapv9OQGGariiiXP9jyu0wQ@mail.gmail.com>
@ 2019-01-01  0:00     ` Mark Wielaard
  2019-01-01  0:00     ` _stati64 patch (Was: [PATCH] Fix include path separator) Mark Wielaard
  1 sibling, 0 replies; 11+ messages in thread
From: Mark Wielaard @ 2019-01-01  0:00 UTC (permalink / raw)
  To: Joshua Watt; +Cc: bzip2-devel

Hi Joshua,

[Note that the mailinglist doesn't accept HTML emails, so you message
isn't in the archives.]

On Wed, Jul 03, 2019 at 03:49:45PM -0500, Joshua Watt wrote:
> On Wed, Jul 3, 2019, 2:04 PM Mark Wielaard <mark@klomp.org> wrote:
> > I think this makes total sense. And I see something similar is already
> > on the 1.1.x branch. But I am slightly hesitant making any build
> > changes. Especially since we don't have any buildbots for Windows. And
> > I personally don't have any access to Windows.
> >
> 
> I did tests with msvc 18.0 (visual studio 2013), and I can probably get
> some newer versions if you want. I can also try with mingw gcc (on Windows;
> already verified on Linux) if you like... I'm not sure what else would be a
> relevant test

Newer compiler are likely to accept it if older ones did.
It has been there since almost the beginning, bzip2-0.9.0c, 1998.
So maybe at some time there were compilers that needed the backslash?

> > Are you sure all Windows compilers accept the forward instead of
> > backwards slash for standard include statements?
> > Is there anything in the C standard that say a forward (or backwards)
> > slash should be accepted?
> >
> 
> Not AFAIK. FWIW, windows itself doesn't care about the slash direction for
> anything except UNC paths, so unless the compilers have really bad path
> parsing logic, it shouldn't matter.

Actually I found the following in the C89 standard about #include
directive parsing:

   If the characters ', \ , , or /* occur in the sequence between the 
   < and > delimiters, the behavior is undefined.

So, it seems accepting \ instead of / is actually an extension.

> > Sorry for being a little pedantic when it comes to build changes.
> >
> 
> It's understandable :)

Maybe a little too pedantic. I cannot really find any references for
sys\stat.h except where sys/stat.h is also accepted. The code mentions
the lcc compiler and MS Visual C compiler. As far as I can see
(quickly reading the documentation) the lcc compile accepts
sys/stat.h. And you tested the Microsoft visual C compiler (and
MINGW). It also moves us closer to the experimental 1.1.x branch.

So lets just accept it. Pushed.

Thanks,

Mark

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: _stati64 patch (Was: [PATCH] Fix include path separator)
  2019-01-01  0:00         ` Mark Wielaard
@ 2019-01-01  0:00           ` Joshua Watt
  2019-01-01  0:00             ` Mark Wielaard
  0 siblings, 1 reply; 11+ messages in thread
From: Joshua Watt @ 2019-01-01  0:00 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: bzip2-devel, Phil Ross


On 7/9/19 4:11 PM, Mark Wielaard wrote:
> Hi Joshua,
>
> On Tue, 2019-07-09 at 10:00 -0500, Joshua Watt wrote:
>> On 7/5/19 3:38 AM, Mark Wielaard wrote:
>>> There is another Windows specific patch which we haven't applied yet
>>> to the 1.0.x branch because we don't have Windows builders/testers.
>>>
>>> It is the attached patch from Phil Ross (CCed) to use _stati64 instead
>>> of _stat to support large >4GB files on Windows.
>>>
>>> If you could test the above works on your setups that would be
>>> appreciated.  Do we need any checks to see whether _stati64 is
>>> available? Or can it be used unconditionally as in this patch?
>> The patch compiles without issues using msvc 18.0 (Visual Studio 2013)
>> and 64-bit MinGW gcc 7.3.0. 'make test' passes also.
> Thanks for the extra testing. Lets apply this patch then.
> It would be good to be able to handle large files on Windows too.
>
>> The "patch in a patch" format was a little confusing at first :)
> Oops, sorry, I wanted to make sure I cherry-picked the original patch
> from Phil, but in doing so, I got the patch-in-patch variant. I applied
> the straight diff one (as attached) so that the source matches what was
> applied on the 1.1.x branch.
>
> BTW. I don't know much about windows, and I assume it doesn't ship with
> bash. But maybe you could take a peek at the bzip2-tests repository and
> see if you could somehow make that work on Windows?
> https://sourceware.org/git/bzip2-tests.git

Windows isn't really my preferred OS either; I'm tasked with maintaining 
an embedded Linux cross compiling environment for Windows as part of my 
day job, so I end up dealing with it more that I would perhaps like :)

Anyway, the good news is that you don't really need to make any changes 
to the bzip2-tests repo; it works fine on Windows as is. As previously 
stated, I compiled bzip2 using MinGW (http://www.mingw.org/). There is 
also a related project called msys (http://www.mingw.org/wiki/MSYS) that 
will give you a traditional bash shell (as well as most other standard 
utilities) in Windows. The run-tests.sh script works just fine there and 
all the tests pass, using a build of bzip2 from b07b105 ("Accept as many 
selectors as the file format allows.").


>
> Cheers,
>
> Mark

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: _stati64 patch (Was: [PATCH] Fix include path separator)
  2019-01-01  0:00               ` Joshua Watt
@ 2019-01-01  0:00                 ` Mark Wielaard
  2019-01-01  0:00                   ` Joshua Watt
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Wielaard @ 2019-01-01  0:00 UTC (permalink / raw)
  To: Joshua Watt; +Cc: bzip2-devel, Phil Ross

Hi Joshua,

On Mon, 2019-07-15 at 08:58 -0500, Joshua Watt wrote:
> On 7/14/19 4:23 PM, Mark Wielaard wrote:
> > 
> > That is really awesome. Thanks so much for testing that out.
> > Can a cross build bzip2 using MinGw (and msys) be run under Wine?
> > That might give us a build CI pipeline for testing bzip2 using the
> > buildbot. It might not be identical to running under actual Windows.
> > But it might be scripted/automated.
> 
> We do something similar to that to test our MinGW SDKs for the Yocto 
> Project. We cross compile the SDK using the MinGW toolchain on Linux 
> (note that MSYS is not required; the Linux system already has all the 
> POSIX tools), then run a set of automated tests under Wine.

OK nice. So for testing you can just use the normal scripts/tools, just
make sure the binary itself is ran through wine?

> You might be able to take this approach also. It wouldn't verify that 
> you can actually compile under Windows (as stated, it is a cross compile 
> from Linux), but it would give some amount of confidence that you can 
> actually run the cross compiled bzip2 on Windows and it will pass the 
> tests. You are correct that Wine isn't a fully faithful reproduction of 
> Windows; I've found a few things that just refuse to run properly under 
> wine, but do fine on actual Windows.

Right. We only want to test that it builds as if for Windows. That is
test the code with BZ_UNIX 0 and BZ_LCCWIN32 1 compiles correctly.

> Most distros have a ready-made MinGW GCC compiler that can be installed. 
> The harder part is getting any additional dependencies. Most of the 
> distros I've seen don't have very many of the MinGW cross compiled 
> library (why would they?) so if your trying to cross compile a large 
> codebase, you can easily get into trouble with missing dependencies. The 
> Yocto project has an advantage in this regard because we are already 
> compiling everything from source anyway, so cross compiling the 
> requisite dependencies for MinGW isn't any additional work. I think that 
> bzip2 might also be able to be cross compiled easily because it doesn't 
> have very many dependencies.

As far as I know bzip2 really shouldn't have any dependencies except
for what is provided by MinGW GCC itself.

> If you want to verify that you can compile under Windows, you might be 
> able to install MSVC in wine and use it to build bzip2. You *might* also 
> be able to install the Windows version of MinGW and MSYS in wine. I 
> don't know how stable this would be or if it would work at all.

Both may go a bit too far. Just being able to (cross) compile to a
Windows binary and then check it works as expected seems all that is
really necessary to make sure we don't break thing (too much).

Lets see... So I installed mingw32-gcc and wine for my distro.

$ make CC=i686-w64-mingw32-gcc
[... mostly seems to work ...]
i686-w64-mingw32-gcc -Wall -Winline -O2 -g -D_FILE_OFFSET_BITS=64  -o
bzip2 bzip2.o -L. -lbz2
./libbz2.a: error adding symbols: Archive has no index; run ranlib to
add one
collect2: error: ld returned 1 exit status
make: *** [bzip2] Error 1

OK, that is odd, but lets do what it says:
$ i686-w64-mingw32-ranlib ./libbz2.a

$ i686-w64-mingw32-gcc -Wall -Winline -O2 -g -D_FILE_OFFSET_BITS=64 \
                       -o bzip2 bzip2.o -L. -lbz2

$ file bzip2
./bzip2: PE32 executable (console) Intel 80386, for MS Windows

Nice!

But...
$ wine ./bzip2
wine: Bad EXE format for Z:\home\mark\src\bzip2-work\bzip2..

OK, apparently I needed mingw64-gcc.

So, same as above with make CC=i686-w64-mingw32-gcc
x86_64-w64-mingw32-ranlib ./libbz2.a and
x86_64-w64-mingw32-gcc -Wall -Winline -O2 -g -D_FILE_OFFSET_BITS=64  -o 
bzip2 bzip2.o -L. -lbz2

$ file bzip2
bzip2: PE32+ executable (console) x86-64, for MS Windows

$ wine ./bzip2 --help
bzip2, a block-sorting file compressor.  Version 1.0.8, 13-Jul-2019.

   usage: bzip2 [flags and input files in any order]

   -h --help           print this message
   -d --decompress     force decompression
   -z --compress       force compression
   -k --keep           keep (don't delete) input files
   -f --force          overwrite existing output files
   -t --test           test compressed file integrity
   -c --stdout         output to standard out
   -q --quiet          suppress noncritical error messages
   -v --verbose        be verbose (a 2nd -v gives more)
   -L --license        display software version & license
   -V --version        display software version & license
   -s --small          use less memory (at most 2500k)
   -1 .. -9            set block size to 100k .. 900k
   --fast              alias for -1
   --best              alias for -9

   If invoked as `bzip2', default action is to compress.
              as `bunzip2',  default action is to decompress.
              as `bzcat', default action is to decompress to stdout.

   If no file names are given, bzip2 compresses or decompresses
   from standard input to standard output.  You can combine
   short flags, so `-v -4' means the same as -v4 or -4v, &c.


That is so awesome! Even though I am not exactly sure what I did.
But it seems possible with some tweaks to get this working.

Could you share some hints and tips on your setup? Or simply explain
what went wrong in the above or what accidentally worked and why?

Also is there a "command line wine"?
The first time you run wine it seems to spawn all kinds of windows
setup thingies. That would work on the buildbots of course.

Thanks,

Mark

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: _stati64 patch (Was: [PATCH] Fix include path separator)
  2019-01-01  0:00       ` Joshua Watt
@ 2019-01-01  0:00         ` Mark Wielaard
  2019-01-01  0:00           ` Joshua Watt
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Wielaard @ 2019-01-01  0:00 UTC (permalink / raw)
  To: Joshua Watt; +Cc: bzip2-devel, Phil Ross

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

Hi Joshua,

On Tue, 2019-07-09 at 10:00 -0500, Joshua Watt wrote:
> On 7/5/19 3:38 AM, Mark Wielaard wrote:
> > There is another Windows specific patch which we haven't applied yet
> > to the 1.0.x branch because we don't have Windows builders/testers.
> > 
> > It is the attached patch from Phil Ross (CCed) to use _stati64 instead
> > of _stat to support large >4GB files on Windows.
> > 
> > If you could test the above works on your setups that would be
> > appreciated.  Do we need any checks to see whether _stati64 is
> > available? Or can it be used unconditionally as in this patch?
> 
> The patch compiles without issues using msvc 18.0 (Visual Studio 2013) 
> and 64-bit MinGW gcc 7.3.0. 'make test' passes also.

Thanks for the extra testing. Lets apply this patch then.
It would be good to be able to handle large files on Windows too.

> The "patch in a patch" format was a little confusing at first :)

Oops, sorry, I wanted to make sure I cherry-picked the original patch
from Phil, but in doing so, I got the patch-in-patch variant. I applied
the straight diff one (as attached) so that the source matches what was
applied on the 1.1.x branch.

BTW. I don't know much about windows, and I assume it doesn't ship with
bash. But maybe you could take a peek at the bzip2-tests repository and
see if you could somehow make that work on Windows?
https://sourceware.org/git/bzip2-tests.git

Cheers,

Mark

[-- Attachment #2: 0001-Fix-a-not-a-normal-file-error-when-compressing-large.patch --]
[-- Type: text/x-patch, Size: 981 bytes --]

From 13d8bce0393ca21cbca1e8ba7692466a64fd46dd Mon Sep 17 00:00:00 2001
From: Phil Ross <phil.ross@gmail.com>
Date: Tue, 21 May 2019 20:46:14 +0100
Subject: [PATCH] Fix a 'not a normal file' error when compressing large files.

The bzip2 command line would report 'not a normal file' for files of
size larger than 2^32 - 1 bytes.

Patch bzip2.c to use _stati64 instead of _stat so that a successful
result is returned for large files.

Resolves https://github.com/philr/bzip2-windows/issues/3.
---
 bzip2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bzip2.c b/bzip2.c
index be3b3be..76c6c94 100644
--- a/bzip2.c
+++ b/bzip2.c
@@ -132,8 +132,8 @@
 
 #   define NORETURN       /**/
 #   define PATH_SEP       '\\'
-#   define MY_LSTAT       _stat
-#   define MY_STAT        _stat
+#   define MY_LSTAT       _stati64
+#   define MY_STAT        _stati64
 #   define MY_S_ISREG(x)  ((x) & _S_IFREG)
 #   define MY_S_ISDIR(x)  ((x) & _S_IFDIR)
 
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] Fix include path separator
@ 2019-01-01  0:00 Joshua Watt
  2019-01-01  0:00 ` Mark Wielaard
  0 siblings, 1 reply; 11+ messages in thread
From: Joshua Watt @ 2019-01-01  0:00 UTC (permalink / raw)
  To: bzip2-devel; +Cc: Joshua Watt

Changes the include path separator for Windows builds to use "/" instead
of "\". Windows has no problems with using a forward slash as a path
separator, but using a backslash causes problems when attempting to
cross compile for other platforms (for example, when trying to cross
compile for MinGW from Linux).
---
 bzip2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bzip2.c b/bzip2.c
index e362c65..be3b3be 100644
--- a/bzip2.c
+++ b/bzip2.c
@@ -128,7 +128,7 @@
 #if BZ_LCCWIN32
 #   include <io.h>
 #   include <fcntl.h>
-#   include <sys\stat.h>
+#   include <sys/stat.h>
 
 #   define NORETURN       /**/
 #   define PATH_SEP       '\\'
-- 
2.21.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: _stati64 patch (Was: [PATCH] Fix include path separator)
  2019-01-01  0:00             ` Mark Wielaard
@ 2019-01-01  0:00               ` Joshua Watt
  2019-01-01  0:00                 ` Mark Wielaard
  0 siblings, 1 reply; 11+ messages in thread
From: Joshua Watt @ 2019-01-01  0:00 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: bzip2-devel, Phil Ross


On 7/14/19 4:23 PM, Mark Wielaard wrote:
> Hi Joshua,
>
> On Tue, 2019-07-09 at 16:59 -0500, Joshua Watt wrote:
>>> BTW. I don't know much about windows, and I assume it doesn't ship with
>>> bash. But maybe you could take a peek at the bzip2-tests repository and
>>> see if you could somehow make that work on Windows?
>>> https://sourceware.org/git/bzip2-tests.git
>> Windows isn't really my preferred OS either; I'm tasked with maintaining
>> an embedded Linux cross compiling environment for Windows as part of my
>> day job, so I end up dealing with it more that I would perhaps like :)
>>
>> Anyway, the good news is that you don't really need to make any changes
>> to the bzip2-tests repo; it works fine on Windows as is. As previously
>> stated, I compiled bzip2 using MinGW (http://www.mingw.org/). There is
>> also a related project called msys (http://www.mingw.org/wiki/MSYS) that
>> will give you a traditional bash shell (as well as most other standard
>> utilities) in Windows. The run-tests.sh script works just fine there and
>> all the tests pass, using a build of bzip2 from b07b105 ("Accept as many
>> selectors as the file format allows.").
> That is really awesome. Thanks so much for testing that out.
> Can a cross build bzip2 using MinGw (and msys) be run under Wine?
> That might give us a build CI pipeline for testing bzip2 using the
> buildbot. It might not be identical to running under actual Windows.
> But it might be scripted/automated.

We do something similar to that to test our MinGW SDKs for the Yocto 
Project. We cross compile the SDK using the MinGW toolchain on Linux 
(note that MSYS is not required; the Linux system already has all the 
POSIX tools), then run a set of automated tests under Wine.


You might be able to take this approach also. It wouldn't verify that 
you can actually compile under Windows (as stated, it is a cross compile 
from Linux), but it would give some amount of confidence that you can 
actually run the cross compiled bzip2 on Windows and it will pass the 
tests. You are correct that Wine isn't a fully faithful reproduction of 
Windows; I've found a few things that just refuse to run properly under 
wine, but do fine on actual Windows.


Most distros have a ready-made MinGW GCC compiler that can be installed. 
The harder part is getting any additional dependencies. Most of the 
distros I've seen don't have very many of the MinGW cross compiled 
library (why would they?) so if your trying to cross compile a large 
codebase, you can easily get into trouble with missing dependencies. The 
Yocto project has an advantage in this regard because we are already 
compiling everything from source anyway, so cross compiling the 
requisite dependencies for MinGW isn't any additional work. I think that 
bzip2 might also be able to be cross compiled easily because it doesn't 
have very many dependencies.


If you want to verify that you can compile under Windows, you might be 
able to install MSVC in wine and use it to build bzip2. You *might* also 
be able to install the Windows version of MinGW and MSYS in wine. I 
don't know how stable this would be or if it would work at all.

> Thanks,
>
> Mark

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: _stati64 patch (Was: [PATCH] Fix include path separator)
  2019-01-01  0:00           ` Joshua Watt
@ 2019-01-01  0:00             ` Mark Wielaard
  2019-01-01  0:00               ` Joshua Watt
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Wielaard @ 2019-01-01  0:00 UTC (permalink / raw)
  To: Joshua Watt; +Cc: bzip2-devel, Phil Ross

Hi Joshua,

On Tue, 2019-07-09 at 16:59 -0500, Joshua Watt wrote:
> > BTW. I don't know much about windows, and I assume it doesn't ship with
> > bash. But maybe you could take a peek at the bzip2-tests repository and
> > see if you could somehow make that work on Windows?
> > https://sourceware.org/git/bzip2-tests.git
> 
> Windows isn't really my preferred OS either; I'm tasked with maintaining 
> an embedded Linux cross compiling environment for Windows as part of my 
> day job, so I end up dealing with it more that I would perhaps like :)
> 
> Anyway, the good news is that you don't really need to make any changes 
> to the bzip2-tests repo; it works fine on Windows as is. As previously 
> stated, I compiled bzip2 using MinGW (http://www.mingw.org/). There is 
> also a related project called msys (http://www.mingw.org/wiki/MSYS) that 
> will give you a traditional bash shell (as well as most other standard 
> utilities) in Windows. The run-tests.sh script works just fine there and 
> all the tests pass, using a build of bzip2 from b07b105 ("Accept as many 
> selectors as the file format allows.").

That is really awesome. Thanks so much for testing that out.
Can a cross build bzip2 using MinGw (and msys) be run under Wine?
That might give us a build CI pipeline for testing bzip2 using the
buildbot. It might not be identical to running under actual Windows.
But it might be scripted/automated.

Thanks,

Mark

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fix include path separator
  2019-01-01  0:00 [PATCH] Fix include path separator Joshua Watt
@ 2019-01-01  0:00 ` Mark Wielaard
       [not found]   ` <CAJdd5GZfTk8EKeUE_C1keKm+gn=qapv9OQGGariiiXP9jyu0wQ@mail.gmail.com>
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Wielaard @ 2019-01-01  0:00 UTC (permalink / raw)
  To: Joshua Watt, bzip2-devel

Hi Joshua,

On Tue, 2019-07-02 at 15:05 -0500, Joshua Watt wrote:
> Changes the include path separator for Windows builds to use "/" instead
> of "\". Windows has no problems with using a forward slash as a path
> separator, but using a backslash causes problems when attempting to
> cross compile for other platforms (for example, when trying to cross
> compile for MinGW from Linux).

Thanks for the patch.

I think this makes total sense. And I see something similar is already
on the 1.1.x branch. But I am slightly hesitant making any build
changes. Especially since we don't have any buildbots for Windows. And
I personally don't have any access to Windows.

Are you sure all Windows compilers accept the forward instead of
backwards slash for standard include statements?
Is there anything in the C standard that say a forward (or backwards)
slash should be accepted?

Sorry for being a little pedantic when it comes to build changes.

Thanks,

Mark

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-07-16  1:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-01  0:00 [PATCH] Fix include path separator Joshua Watt
2019-01-01  0:00 ` Mark Wielaard
     [not found]   ` <CAJdd5GZfTk8EKeUE_C1keKm+gn=qapv9OQGGariiiXP9jyu0wQ@mail.gmail.com>
2019-01-01  0:00     ` Mark Wielaard
2019-01-01  0:00     ` _stati64 patch (Was: [PATCH] Fix include path separator) Mark Wielaard
2019-01-01  0:00       ` Joshua Watt
2019-01-01  0:00         ` Mark Wielaard
2019-01-01  0:00           ` Joshua Watt
2019-01-01  0:00             ` Mark Wielaard
2019-01-01  0:00               ` Joshua Watt
2019-01-01  0:00                 ` Mark Wielaard
2019-01-01  0:00                   ` Joshua Watt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).