GNOME Bugzilla – Bug 667666
No need to deal with CRLF
Last modified: 2016-03-31 13:55:29 UTC
Christophy discovered that windows 7 express install was broken again (perhaps others were borken too but didn't check). This patches fixes it.
Created attachment 204991 [details] [review] No need to deal with CRLF Turns out that using CRLF somehow messes-up the copying of unattended data. Since both Windows XP and 7 don't mind LF, lets just use LF for all OSs.
Created attachment 204992 [details] [review] express,win7: No need to deal with CRLF Turns out that using CRLF somehow messes-up the copying of unattended data. Since both Windows XP and 7 don't mind LF, lets just use LF for all OSs.
Created attachment 204993 [details] [review] express,win7,winxp: No need to deal with CRLF Turns out that using CRLF somehow messes-up the copying of unattended data. Since both Windows XP and 7 don't mind LF, lets just use LF for all OSs.
"somehow" messes up? Do you have more insight as to what's going on?
(In reply to comment #4) > "somehow" messes up? Do you have more insight as to what's going on? No, do you? :)
In my testing, this patch helps to get autoinstall to work (without it autoinstall doesn't work most of the time). No idea if it papers over the bug and if it will come back later or not. It's definitely not fixing the issue with CRLF, it's at best work arounding the problem by going back to a state that seemed to be working.
(In reply to comment #6) > In my testing, this patch helps to get autoinstall to work (without it > autoinstall doesn't work most of the time). Now you are getting me really confused. So you can still reproduce this bug with this patch in just not reliably? Cause "most of the time" would imply that. > No idea if it papers over the bug > and if it will come back later or not. It's definitely not fixing the issue > with CRLF, We don't know what the issue is so how can you say this with such certainty? > it's at best work arounding the problem by going back to a state > that seemed to be working. We were using CRLF for windows and unattended didn't work for either of us. This patch just makes LF everywhere and unattended works after that so how is it a work-around? Just because we don't know the root cause of the problem, doesn't mean the solution is a work-around. You are free to investigate this issue to find the root cause, I have many other things on my todo list and if a solution a. fixes the problem b. doesn't make the code ugly (this patch actually simplifies code) and c. doesn't break anything else, I don't see why such a patch shouldn't be applied immediately.
(In reply to comment #7) > (In reply to comment #6) > > In my testing, this patch helps to get autoinstall to work (without it > > autoinstall doesn't work most of the time). > > Now you are getting me really confused. So you can still reproduce this bug > with this patch in just not reliably? Cause "most of the time" would imply > that. No no, I think you missed the "with*out* it". I've seen valid xml files being generated a few times without the patch. So I assume that it will work sometimes without the patch. So far, I didn't get a failure with the patch > > > No idea if it papers over the bug > > and if it will come back later or not. It's definitely not fixing the issue > > with CRLF, > > We don't know what the issue is so how can you say this with such certainty? > See below for what I meant here > > it's at best work arounding the problem by going back to a state > > that seemed to be working. > > We were using CRLF for windows and unattended didn't work for either of us. > This patch just makes LF everywhere and unattended works after that so how is > it a work-around? Just because we don't know the root cause of the problem, > doesn't mean the solution is a work-around. CRLF was added because it's the native line-ending on Windows so it's more natural to do that. Unfortunately switching to CRLF causes this bug, so you're basically suggesting to revert this CRLF change to "fix" the bug. Fixing the bug would mean still using CRLF and changing XXX which is causing the issue. The fact that we are not doing that but instead we remove a minor cosmetic feature is why I'm talking about a workaround. > > You are free to investigate this issue to find the root cause, I have many > other things on my todo list and if a solution a. fixes the problem We disagree on what "fixing" means. All we know is that so far we didn't get the bug with this patch. There is 50% chance that the bug will come back in a slightly different form after some unrelated code changes.
Ok so after a few hours of investigation, I believe I found out the root cause of this issue. There was a missing 'yield' keyword (src/unattended-installer.vala:284) that caused a race-condition: We were carrying on with domain creation/installation before waiting for the file to be copied to the floppy first. Instead of just fixing that, I think I did something even better: I removed the need for the floppy image all together. See bug#67895 for details.