Changeset View
Standalone View
plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp
| Show First 20 Lines • Show All 252 Lines • ▼ Show 20 Line(s) | 252 | { | |||
|---|---|---|---|---|---|
| 253 | return IDefinesAndIncludesManager::CompilerSpecific; | 253 | return IDefinesAndIncludesManager::CompilerSpecific; | ||
| 254 | } | 254 | } | ||
| 255 | 255 | | |||
| 256 | CompilerPointer CompilerProvider::defaultCompiler() const | 256 | CompilerPointer CompilerProvider::defaultCompiler() const | ||
| 257 | { | 257 | { | ||
| 258 | if (m_defaultProvider) | 258 | if (m_defaultProvider) | ||
| 259 | return m_defaultProvider; | 259 | return m_defaultProvider; | ||
| 260 | 260 | | |||
| 261 | auto rt = ICore::self()->runtimeController()->currentRuntime(); | 261 | auto rt = ICore::self()->runtimeController()->currentRuntime(); | ||
| 262 | const auto path = QFile::decodeName(rt->getenv("PATH")).split(QDir::listSeparator()); | | |||
| 263 | 262 | | |||
apol: If you want to keep stored the absolute path, this logic will need to be moved with the new… | |||||
Sorry, I didn't get your comment. That path variable is unused now and can be removed. arrowd: Sorry, I didn't get your comment. That `path` variable is unused now and can be removed. | |||||
apol: you didn't port what it did to the new code. | |||||
I'd really help me, if you tell exactly what should I fix, since I don't really understand that runtime stuff. I assumed that I need to check for presence of compiler executable in runtime PATH and fixed that. Please corrent me if I'm wrong. arrowd: I'd really help me, if you tell exactly what should I fix, since I don't really understand that… | |||||
| 264 | for ( const CompilerPointer& compiler : m_compilers ) { | 263 | for ( const CompilerPointer& compiler : m_compilers ) { | ||
| 265 | const bool absolutePath = QDir::isAbsolutePath(compiler->path()); | 264 | if (rt->findExecutable(QFileInfo(compiler->path()).fileName()).isEmpty()) | ||
remove the QFileInfo and .fileName() here, just use the path like previously. note how the path could be an absolute path not inside the PATH, your patch would break this mwolff: remove the `QFileInfo` and `.fileName()` here, just use the path like previously. note how the… | |||||
mwolff: see this comment again | |||||
In other comment you said
So, if compiler->path() can return absolute path, is it OK to pass it as is into rt->findExecutable()? arrowd: In other comment you said
> afaik it's called path since it can be a user-provided absolute… | |||||
Yes, an absolute path will then just be resolved directly within the RT without looking at the PATH env var of the RT mwolff: Yes, an absolute path will then just be resolved directly within the RT without looking at the… | |||||
| 266 | if ((absolutePath && QFileInfo::exists(rt->pathInHost(Path(compiler->path())).toLocalFile())) | | |||
@apol You worked with runtime thingie more than me. What does this check do? Shouldn't it be ! QFileInfo::exists()? arrowd: @apol You worked with runtime thingie more than me. What does this check do? Shouldn't it be `! | |||||
Sounds likely. It's being removed anyway here, right? Actually this whole patch will break it because it's checking on the host system and not on the runtime. apol: Sounds likely. It's being removed anyway here, right?
Actually this whole patch will break it… | |||||
arrowd: I've brought this check back and inverted the condition. | |||||
you are right - the issue is that the old code skips compilers when they exist - there's a ! missing in front of the parens here probably mwolff: you are right - the issue is that the old code skips compilers when they exist - there's a ! | |||||
| 267 | || QStandardPaths::findExecutable( compiler->path(), path).isEmpty() ) { | | |||
| 268 | continue; | 265 | continue; | ||
| 269 | } | | |||
| 270 | | ||||
| 271 | m_defaultProvider = compiler; | 266 | m_defaultProvider = compiler; | ||
| 272 | break; | 267 | break; | ||
| 273 | } | 268 | } | ||
| 274 | if (!m_defaultProvider) | 269 | if (!m_defaultProvider) | ||
| 275 | m_defaultProvider = createDummyCompiler(); | 270 | m_defaultProvider = createDummyCompiler(); | ||
| 276 | 271 | | |||
| 277 | qCDebug(DEFINESANDINCLUDES) << "new default compiler" << rt->name() << m_defaultProvider->name() << m_defaultProvider->path(); | 272 | qCDebug(DEFINESANDINCLUDES) << "new default compiler" << rt->name() << m_defaultProvider->name() << m_defaultProvider->path(); | ||
| 278 | return m_defaultProvider; | 273 | return m_defaultProvider; | ||
| ▲ Show 20 Lines • Show All 55 Lines • Show Last 20 Lines | |||||
If you want to keep stored the absolute path, this logic will need to be moved with the new QStandardPaths::findProgram()