-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
cin.seekg(0) erroneously returns success in 2.41.0 when cin is a Git Bash pipe. Worked in all earlier versions. #4464
Comments
This might be a fallout from git-for-windows/msys2-runtime@0e357e2. Looking forward to that minimal reproducer so that we can know for sure. |
Here is a minimal reproducer. First is a bash script that finds the size of the target file and runs the pipe, sending the pipe-reader program the size of file to be used for comparison. #!/bin/bash
FILENAME=$1
FILESIZE=$(stat -c%s "$FILENAME") # Linux, Git 4 Windows
#FILESIZE=$(stat -f%z "$FILENAME") # macOS, BSD
echo "Size of $FILENAME = $FILESIZE bytes."
cat $FILENAME | ./test-pipe.exe $FILESIZE and a C++ program that reads from the pipe into a stringstream buffer with an istream #include <iostream>
#include <sstream>
#include <assert.h>
#include <stdlib.h>
#if defined(_WIN32)
#include <corecrt_io.h>
#include <fcntl.h>
#include <windows.h>
#endif
using namespace std;
int main(int argc, char* argv[])
{
size_t filesize = atoi(argv[1]);
istream* isp;
stringstream buffer;
#if defined(_WIN32)
/* Set "stdin" to have binary mode */
(void)_setmode( _fileno( stdin ), _O_BINARY );
#endif
// Can we seek in this cin?
cin.seekg(0);
if (cin.fail()) {
// Read entire file into a stringstream so we can seek.
buffer << std::cin.rdbuf();
isp = &buffer;
} else {
isp = &cin;
}
uint8_t dbuf[80];
isp->read((char*)dbuf, sizeof(dbuf));
off_t dataStart = (off_t)(isp->tellg());
assert(dataStart == sizeof(dbuf) && "In pointer not at expected offset");
isp->seekg(0, ios_base::end);
if (isp->fail()) {
cerr << "Seek to end failed" << endl;
exit(1);
}
off_t dataSizeInFile = (off_t)(isp->tellg());
if (dataSizeInFile < 0) {
cerr << "File tell failure" << endl;
exit(1);
}
if (dataSizeInFile != filesize) {
cerr << "End of pipe(" << dataSizeInFile << ") != filesize(" << filesize << ")." << endl;
exit(1);
}
cout << "All good." << endl;
return 0;
} Compile the C++ program in a "developer PowerShell" or any shell that has Visual Studio stuff in PATH and environment
It probably does not matter which compiler. I chose Run the script in Git Bash $ ./test-pipe ../tests/testimages/color_grid_uastc_zstd.ktx2
Size of ../tests/testimages/color_grid_uastc_zstd.ktx2 = 170512 bytes.
End of pipe(126976) != filesize(170512). Note that there is no testing of the arguments in either script or c++ program. The former expects one argument, the path to the file to cat. The latter expects a single argument that it runs atoi on. It does not check the result. |
Hey @dscho, is there any update on the status of this issue? |
$ ./test-pipe ../tests/testimages/color_grid_uastc_zstd.ktx2 |
@MarkCallow thank you for the reproducer. I adapted this minimally to compile with test-pipe.cxx#include <iostream>
#include <sstream>
#include <assert.h>
#include <stdlib.h>
#if defined(_WIN32)
#ifdef __MSVC__
#include <corecrt_io.h>
#else
#include <cstdint>
#endif
#include <fcntl.h>
#include <windows.h>
#endif
using namespace std;
int main(int argc, char* argv[])
{
off_t filesize = (off_t)strtoull(argv[1], NULL, 10);
istream* isp;
stringstream buffer;
#if defined(_WIN32)
/* Set "stdin" to have binary mode */
(void)_setmode( _fileno( stdin ), _O_BINARY );
#endif
// Can we seek in this cin?
cin.seekg(0);
if (cin.fail()) {
// Read entire file into a stringstream so we can seek.
buffer << std::cin.rdbuf();
isp = &buffer;
} else {
isp = &cin;
}
uint8_t dbuf[80];
isp->read((char*)dbuf, sizeof(dbuf));
off_t dataStart = (off_t)(isp->tellg());
assert(dataStart == sizeof(dbuf) && "In pointer not at expected offset");
isp->seekg(0, ios_base::end);
if (isp->fail()) {
cerr << "Seek to end failed" << endl;
exit(1);
}
off_t dataSizeInFile = (off_t)(isp->tellg());
if (dataSizeInFile < 0) {
cerr << "File tell failure" << endl;
exit(1);
}
if (dataSizeInFile != filesize) {
cerr << "End of pipe(" << dataSizeInFile << ") != filesize(" << filesize << ")." << endl;
exit(1);
}
cout << "All good." << endl;
return 0;
} The good news is that it reproduces even with vanilla Cygwin. The bad news that it reproduces even with their Since I lack the bandwidth to pursue this further (and unfortunately, I suspect that the pseudo Console code is responsible for this bug even if pipes have exactly nothing at all to do with pseudo Consoles, and that code is really, really obtuse and there's nothing I can do about it), I would like to suggest taking this to the Cygwin project who are in a much better position to fix this bug. See https://cygwin.com/problems.html how to report bugs there. |
I've reported the issue to the Cygwin project which has lead to an interesting discussion. One question that has arisen is whether the failure has been seen when the pipe reader is a Cygwin program. @dscho how did you run the tests on cygwin. Did you compile |
No, I specifically compiled it using mingw-w64 g++. This is definitely a bug when calling non-Cygwin consumers. |
In the discussion on the cygwin@cygwin.com mailing list we have identified the exact cause of this issue. The trigger is the This can be worked around by always buffering stdin data on Windows. I will change the issue title to match the understanding. |
This issue is about a problem with pipes in the Bash / MingW(?) / MSYS(?) included with 2.41.0. Is this the right place to report such an issue? If not, where should I report it and how to I find the relevant version information to included with the report?
Setup
defaults?
to the issue you're seeing?
No. I've seen it on my own machine and on 2 different CI runners.
Details
Bash and PowerShell.
Minimal, Complete, and Verifiable example
this will help us understand the issue.
$ cat color_grid_uastc_zstd.ktx2 | ktx2check
I have not yet been able to create a simple reproducer
I expected
ktx2check
to tell me the file was valid.ktx2check
told me the file did not have the expected byte count of image data and then told me it wasn't a KTX file.URL to that repository to help us with testing?
This is not repo related.
What this is really all about
Now the, in this instance not very helpful, boilerplate is out of the way here are the gory details.
Pipes between bash processes are not working correctly. New in 2.41.0. All worked fine in Git for Windows 2.40.1.windows.1 and all previous that I've used. The problem appeared in our GitHub workflows builds on the VS2019 and VS 2022 runners after Git was updated to 2.41.0.
A simple
cat foo | cat > bar
works fine.bar
ends up identical tofoo
. It is more complicated than that. My applicationktx2check
is doing thisReads after this all work fine. Once the app has read everything up to payload data it does
The tellg result shows the size is significantly less than the actual file data. 43k less in a 170k file. It is seemingly being truncated somewhere.
Later the app does
and starts reading from _streambuf. All data read from the streambuf is gibberish leading to the not a KTX file message.
The application code makes no distinction between a pipe and stdin redirection from a file. It just uses std::cin. stdin redirection still works.
I won't have time to try to create a minimal reproducer for several more days. Sorry for that. I am hoping somebody else will recognize the symptoms.
The text was updated successfully, but these errors were encountered: