Andrew Fish [mailto:[email protected]] wrote:
On Sep 15, 2014, at 8:51 PM, Scott Duplichan <[email protected]
<mailto:[email protected]> > wrote:
Jordan Justen [ <mailto:[email protected]> mailto:[email protected]] wrote:
]On Mon, Sep 15, 2014 at 5:38 PM, Scott Duplichan < <mailto:[email protected]>
[email protected]> wrote:
]> Jordan Justen [ <mailto:[email protected]> mailto:[email protected]] wrote:
]>
]> ]I think in most cases our EDK II code base could compile cleanly with
]> ]these toolchains if minor changes were made to the source. For
]> ]example, I sent a patch in February:
]> ]"OvmfPkg: Fix VS2005 compiler warnings"
]> ]and, you actually commented on the thread.
]> ]
]> ]Anyway, I don't work in windows too often, and I ended up not
]> ]finishing cleaning up that patch and getting it upstream.
]> ]
]> ]But, generally, I think this is the approach that we prefer to dealing
]> ]with compiler warnings. Can we clean up warnings in these packages
]> ]through code rework instead of disabling warnings?
]>
]> From my point of view there are a couple of disadvantages to this approach.
]> The patch I submitted is a permanent fix. The code rework method requires
]> on-going work. That is, even if code changes can make the build pass today,
]> future code rework will be required as the project evolves. That is because
]> few developers use the old Microsoft tools. They will submit code that builds
]> cleanly with newer tools. But eventually someone will build with the old
]> tools and encounter one of these problems. That will require new code rework.
]> A more fundamental objection to me is the idea of changing working, portable,
]> bug-free code to work around the 'broken warnings' of old build tools. If
]> newer build tools didn't exist, then working around these warning limitations
]> might make sense. But newer build tools do exist, and in fact most developers
]> use them. I won't object further if someone else wants to change the code
]> for this reason, but I don't want my name on such a change.
]
]This would be a major change to how EDK II addresses these kinds of issues.
]
]EDK II (and EDK before that) has always pushed for a high number of
]warnings and caused a build errors on warnings.
]
]This strategy goes way back. I don't even know when it started. Maybe
]Vincent or Andrew know.
]
]At any rate, it does indeed cause an ongoing drag, but changing this
]has never been seriously considered. Maybe this is as much due to
]momentum as anything. :)
]
]But, I don't think you've made the case that disabling these warnings
]will end up producing better code. (Personally, I couldn't say one way
]or the other.)
Hello Jordan,
The problem this patch set addresses is broken x86 builds using Windows
tool chains. To say 5 of the 9 supported Windows tool chains are broken
is not something to be proud of:
DDK3790 broken
VS2003 broken
VS2005 broken
VS2008 working
VS2010 working
VS2012 working
VS2013 broken
Cygwin broken (I have been told)
Intel ?
The patch set is intended to make currently broken Windows tool chains
usable, without making the code worse. In fact, this patch set contains
zero code changes. That makes it unlikely to break any previously working
build.
Employees of big companies like Intel Dell, HP, etc have site licenses to
Microsoft build tools. But what about smaller companies? Many readers of
this list have never worked for a small company. Microsoft compilers cost
several hundred US dollars per license. That kind of expense is hard to get
approved from a small company. How do you tell your manager your group needs
a set of new Visual Studio licenses because the UEFI guys at Intel won't fix
their build problems? Intel is saying F*** Y** to the small developer.
Well gcc and clang are supported too.
That is great and I am happy to see it. But the problem is gcc support for EDK2
builds is not currently available for Windows. The
IT department of a company such as HP supports Windows machines only. You are
free to bring in your own Linux box. But that gets
inconvenient. It is so much easier to do everything on one machine.
I am pretty sure this problem of broken tool chains will never get fixed.
Even if someone who has all the Microsoft tools fixes the breakage by putting
in a bunch of code workarounds, it won't stay fixed. That is because the
current policy of enabling /WX on old Microsoft tool chains is clearly not
workable.
Please see my response to Jordan. All it takes is some one testing all the
"supported" toolchains.
I agree with what you said.
The only workable solution is to do like coreboot and have only one official
tool chain. Then disable all troublesome warnings on all other tool chains.
Is there any other solution? I think not.
I don't know as I don't understand your data?
What is an "e" build? or a build command without a command?
A file with:
e -p D:\uefi\buildtest\edk2\OvmfPkg\OvmfPkgIA32.dsc -b DEBUG -t VS2003 -a IA32
-n 16 -DSECURE_BOOT_ENABLE -DFD_SIZE_2MB
...
-p D:\uefi\buildtest\edk2\ShellPkg\ShellPkg.dsc -b RELEASE -t DDK3790 -a IA32
-n 16
build.exe -p D:\uefi\buildtest\edk2\AppPkg\AppPkg.dsc -b DEBUG -t DDK3790 -a
X64 -n 16
build.exe -p D:\uefi\buildtest\edk2\AppPkg\AppPkg.dsc -b RELEASE -t DDK3790 -a
X64 -n 16
...
Sorry about that. I accidentally truncated some text when removing the path
from in front of the build command. Every line should
start with "build.exe -p":
build.exe -p D:\uefi\buildtest\edk2\OvmfPkg\OvmfPkgIA32.dsc -b DEBUG -t VS2003
-a IA32 -n 16 -DSECURE_BOOT_ENABLE -DFD_SIZE_2MB
...
build.exe -p D:\uefi\buildtest\edk2\ShellPkg\ShellPkg.dsc -b RELEASE -t DDK3790
-a IA32 -n 16
build.exe -p D:\uefi\buildtest\edk2\AppPkg\AppPkg.dsc -b DEBUG -t DDK3790 -a
X64 -n 16
build.exe -p D:\uefi\buildtest\edk2\AppPkg\AppPkg.dsc -b RELEASE -t DDK3790 -a
X64 -n 16
Your list of warning don't correlate to tool chains? So that makes it hard to
understand what is going on. Can you provide a list of
warning per toolchain?
OK, here are the complete log files: http://notabs.org/edk2/build-logs.7z.
Directory 'patched' shows the successful builds after
patching. Directory 'unpatched-noWX' is svn version 16101 build results, but
with Microsoft -WX (treat warnings as errors) removed.
I removed that temporarily to make all warnings visible. Directory
'patched-noopt' is a new test that shows a few new failures when
a NOOPT build is made. The NOOPT is useful for debugging with Microsoft builds
because Microsoft debug info supplies information
about local variables only when they are stored in memory.
Thanks,
Scott
Thanks,
Andrew Fish
I guess the big difference of opinion here is about the significance of 5
broken tool chains. I imagine some of these have been broken for years.
What kind of first impression does that make on someone new to EDK2 when he
downloads it and tries to build it?
------------------------------------------------------------------------------
Want excitement?
Manually upgrade your production database.
When you want reliability, choose Perforce.
Perforce version control. Predictably reliable.
http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel