From kde-buildsystem Mon Apr 13 00:42:11 2009 From: Michael Witten Date: Mon, 13 Apr 2009 00:42:11 +0000 To: kde-buildsystem Subject: Re: [PATCH] bug 174806 Message-Id: X-MARC-Message: https://marc.info/?l=kde-buildsystem&m=123958336819732 2009/4/12 Thiago Macieira : > Michael Witten wrote: >>> I repeat once again: unless the user tells us so in the command-line, >>> do not install anything outside the prefix. >> >>I understand this sentiment, and I'm inclined to agree with it, but >> setting PYTHONPATH is not really a suitable answer, because it is >> searched before the default paths, which could be problematic, in >> general. > > Sorry, it has to be. If you don't want to set PYTHONPATH, then you should > pass the argument to cmake and install as root. > > Like I said: configure --prefix=/usr --sysconfigdir=/etc I agree with this policy as a matter of theory. However, I think installing to a known site-packages is much more practical; after all, if you really want things in a nonstandard location (from python's view) like the KDE prefix, you're free to use -DPYTHON_SITE_PACKAGES_INSTALL_DIR=... By default, I suggest it's better to work with python rather than make python work with KDE; at least it would be useful to provide titular modules that can find the real stuff in the KDE prefix. Of course, the problem with using site-packages is that an upgrade of python will require PYTHONPATH anyway to point to the old site-packages (unless people are smart enough to compile python to look there too), or the KDE python bindings will need to be reinstalled for the new site-packages. However, it makes sense that an extraordinary upgrade should require extraordinary measures. In any case, I conceed. I don't really care where these things are installed or what variables must be setup. In fact, the more I think about it, I don't even mind if .py files are byte-compiled to .pyc outside of versioned site-packages, because they will be ignored if they aren't the right version (though I would suggest KDE alert the user of the need to run python's compileall with privileges [or some other setuid tool KDE could provide]). >>> As for the cosmetic changes, it's necessary to address them, since >>> there are a lot of them, which means reviewing your patches is hard. > >> * Repeated args for ELSE, ENDIF, ENDFOR, ENDWHILE, etc. (This is >> a huge mistake in my opinion. I think the entire codebase should >> have that nonsense filtered out; I will one day write a Perl >> script to do exactly that. > > Not your decision to make. While it's mandated, you follow it. But that's > not my point. I care more about progress than I do about worshipping bad policies. But that's not my point. > How about the changes to regexp, changing \\. to [.] ? That's unnecessary. > I'm not saying you shouldn't do it, but do it in a separate patch... you're trying > to do too much in one single patch. Please split it into multiple patches, each > one doing one specific thing. Your request is completely reasonable and is aligned with my modus operandi. However, your request is unreasonable until KDE starts using a real version control system like git. In my case, I wanted to look into a completely different issue, so I checked out the svn trunk and got stalled on this bug; I hadn't intended to provide any extensive patches until I was steeped in this problem, so I never bothered to convert the working directory to a sane system. My patches are pretty clean. As for your example of patch noise, I had already rewritten 70% of the file, so I decided to make the rest of that one file match with my new code by capitalizing the cmake commands (this also matches most KDE cmake code and as well as the cmake code used for cmake itself). Because I had already modified the line, why not make the regular expression a little more sane too? Should I have done this in a separate patch? I would have, if I hadn't been tricked into using something as subpar as svn. >>> example, there's a new file called ParseArgs.cmake which adds a macro >>> that is not used anywhere. >> >>It is used in PythonMacros.cmake's INSTALL_PYTHON, and it was written >> with the intention of being used more. > > It's not in the patch you attached. You must be looking at the wrong patch; though, you missed the python code too... Sincerely, Michael Witten _______________________________________________ Kde-buildsystem mailing list Kde-buildsystem@kde.org https://mail.kde.org/mailman/listinfo/kde-buildsystem