From: Daniel P. <dp...@gm...> - 2015-05-11 21:44:35
|
OK fine. I'm pretty sure this is a bug in MSVC, that the namespace lookup for a templated function happens in the context in which the template is evaluated (as opposed to where it was defined). But it makes sense to patch it anyway. Dan On Mon, May 11, 2015 at 2:09 PM, Jan Trmal <jt...@gm...> wrote: > Ok, I will have a.look tmrw. I think I've resolved the types issue (last one > you mentioned). I generated the patch for openfst, that defines the int > types within the openfst namespace - originally they exist on.the highest > scope (i.e. ::int32) > The patch has something like > namespace fst { > typedef int32 ::int32; > } > > Doping this, the lookup works fine even under the ms c++. > To me, this is optimal solution as we have to patch the openfst anyway (and > my change wont break any code). > You can.have a look ať the openfstwin-1.3.3.patch in tools/extras. > I am open to any suggestion or general discussion on how this should be made > better. > Y. > > On May 11, 2015 9:03 PM, "Daniel Povey" <dp...@gm...> wrote: >> >> > Well, the build was not "painless" at all. In fact, the perl script >> > generated a solution with 600+ projects that simply failed to build. VS was >> > churning out dependencies on these, and crashed in about 10 minutes of just >> > sitting there with a 100% CPU use. Not a single file was compiled, so I >> > think it was just autogenerating dependencies at this point. >> >> Build systems on Windows are pretty broken. I think in Microsoft they >> use some other system, not the one used in VS, which is not publicly >> released. >> >> > I did not try to build from the command line though. Am I understanding >> > it would compile? >> >> No, not everything would compile. Sometimes the VS compiler just crashes. >> >> > What I did not like about the existing system is that I could not plug >> > in CUDA or intel compiler. It is extremely rigid in the option part, and to >> > change compiler options you are most likely to change the perl code that >> > generates the project files, or modify the different MSBuild include files >> > (called "property sheets" in this context) that are interplaying quite >> > non-trivially. No, I have spent a few days on the build system not for a >> > love of tinkering with MSBuils at all! :) >> >> What we committed was just a hack to try to get something working. >> IIRC we just looked at the build files generated by VS to try to guess >> what the format of those files was supposed to be; I'm not sure that >> they are really intended to be human readable/writable (unless that >> human really loves XML). >> >> > Anyway, you can look at it at >> > https://github.com/kkm000/kaldi/tree/winbuild/msbuild. (NB I have not >> > updated the branch in a while) If you think this is not an acceptable >> > solution, I'm just dropping the ball, no offence taken, naturally. I can >> > maintain this as an "unofficial port" for a while. In any case, build is the >> > least problem, as Dan has pointed out already... >> > >> > The biggest (in the number of changed lines) changes I had to make were >> > in the namespace using declarations, because of conflicts between the types >> > int32 and friends from different namespaces. I ended up reading the C++ >> > standard to understand whether gcc, icl or msvc is "correct" in different >> > cases they are complaining about, and the standard appears not to define any >> > behavior there. These changes I would prefer accepted into the mainline of >> > development, because they are harder to maintain in a fork. >> >> In any case where you have to do something like typedef kaldi::int32 >> int32, we should definitely get this committed- talk to Yenda. >> Actually, this is really a bug in OpenFST which they haven't fixed >> yet. There is a "proper" POSIX way to get types like int32, which we >> use in Kaldi, but in OpenFst they do it in a different, ad-hoc way >> which sometimes generates different types (e.g. on a system where int >> and long int are both 32-bit). So the Kaldi int32 and the OpenFst >> int32 end up being different, incompatible types. >> >> Dan >> >> >> >> -----Original Message----- >> >> From: Jan Trmal [mailto:jt...@gm...] >> >> Sent: 2015-05-11 0201 >> >> To: Dan Povey >> >> Cc: Kirill Katsnelson; kal...@li...; kaldi- >> >> us...@li...; 洪孝宗 >> >> Subject: Re: [Kaldi-users] Kaldi compiles now under VS 2013 >> >> >> >> Guys, the migration to git is "unofficially" done -- unless we find >> >> some problems with it, the repo (https://github.com/kaldi- >> >> asr/kaldi.git) will stay as it is now. >> >> >> >> Kirill, there is no reason why wait. We should start working on it. >> >> I propose you could first send the Kaldi code patches -- I actually >> >> worked on this and I'm able to compile Kaldi, so this should be quite >> >> straightforward, provided you will generate the diff against the HEAD. >> >> >> >> Then we can start looking at the "build system". I understand your >> >> reasons and I think they might be legit. My feeling is, however, that a >> >> lot of people are not using MSBuild and will expect the MSVC solution, >> >> just because this is how they usually work. I haven't tested it, but >> >> you should be able to use the current SLN to build the binaries from >> >> command line -- that should be completely painless -- but again, I >> >> haven't tested it. >> >> y. >> >> >> >> >> >> On Thu, May 7, 2015 at 10:53 PM, Daniel Povey <dp...@gm...> wrote: >> >> >> >> >> >> >> To the extent that your patches are simple fixes and minor >> >> changes to >> >> >> the code, I think we could apply them. Perhaps you could work >> >> with >> >> >> Yenda to get your changes checked? >> >> > >> >> > Easy enough, and as easy to hold till the migration is done. >> >> Was there a change in the plans? >> >> >> >> Not really a change in the plans, but we don't have a definite >> >> timeline for migrating to git, and it could be that we'll keep >> >> the svn >> >> as upstream for a while. Your changes seem reasonable, but I >> >> think it >> >> would be better to get things started now rather than later. >> >> The >> >> git/svn issues should not be that hard. Just start sending >> >> patches to >> >> Yenda and we'll handle your changes bit by bit. >> >> Dan >> >> >> >> >> >> >> >> > >> >> >> If you update your git repo to >> >> >> point to the current code, then a patch should be applicable >> >> by the >> >> >> program "patch" to the svn repo too. >> >> > >> >> > There are many changes, and I certainly want to split the patch >> >> so it is readable. One megapatch is not reviewable. >> >> > >> >> >> Regarding your build approach, you could send me and Yenda >> >> some details >> >> >> about how you did it, and we could consider whether to support >> >> that. >> >> > >> >> > There's a Powershell script that takes information from every >> >> src/*/Makefile into a simple declarative MSBuild script >> >> $dirname.kwproj, and a top-level MSBuild "makefile" that drives the >> >> whole thing. A little bit more complex than that to allow for user- >> >> defined options, but generally this is it. This supports building >> >> libraries, tools, tests and running the tests with command line >> >> switches. The whole thing pretty much mimics a linux make process but >> >> using MS tools. >> >> > >> >> >> I don't think it makes sense to try to maintain a parallel >> >> "windows- >> >> >> compatible" version of the scripts, if there are larger >> >> changes >> >> >> required there. >> >> > >> >> > I was targeting for no changes at all. There maybe a few very >> >> small patches that supposed not to break compatibility. >> >> > >> >> >> Anyway you depend on cygwin for scripting, and the set >> >> >> of people who want to run cygwin and build with MSVC is >> >> probably >> >> >> limited enough that I don't think it makes sense for us to try >> >> to >> >> >> support it. >> >> > >> >> > This is not as simple a choice as it seems at first. Alex Hung >> >> just posted a trick to build CUDA under Cygwin; before he did I thought >> >> it was not possible. MKL is another story. I would rather build under >> >> native Windows than try to pull this monster into the Cygwin build >> >> environment. >> >> > >> >> > -kkm >> >> > >> >> >> >> >> > |