Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the devfile to work with c/c++ stack #73

Merged
merged 6 commits into from
Sep 26, 2019
Merged

Add the devfile to work with c/c++ stack #73

merged 6 commits into from
Sep 26, 2019

Conversation

tolusha
Copy link
Contributor

@tolusha tolusha commented Aug 6, 2019

What does this PR do?

It adds the devfile to work with c/c++ stack

What issues does this PR fix or reference?

eclipse-che/che#13698

@tolusha tolusha added the enhancement New feature or request label Aug 6, 2019
@tolusha tolusha self-assigned this Aug 6, 2019
@slemeur slemeur mentioned this pull request Aug 6, 2019
20 tasks
@ibuziuk
Copy link
Member

ibuziuk commented Aug 6, 2019

@tolusha am I correct that this PR is meant to be merged after GA ?

@tolusha
Copy link
Contributor Author

tolusha commented Aug 6, 2019

@ibuziuk yes, you are right. It is for 7.1.0

@tolusha tolusha changed the title Add the devfile to work with c/c++ stack [DON'T MERGE 7.1.0 TARGET] Add the devfile to work with c/c++ stack Aug 6, 2019
@tolusha tolusha changed the title [DON'T MERGE 7.1.0 TARGET] Add the devfile to work with c/c++ stack Add the devfile to work with c/c++ stack Aug 15, 2019
@tolusha
Copy link
Contributor Author

tolusha commented Aug 23, 2019

@l0rd @amisevsk
Could some approve the PR?

Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't seem to get this workspace started on latest nightlies.

On OpenShift, the cpp-plugin container enters a crash loop, terminated with exit code 1.

@amisevsk
Copy link
Contributor

Also worth noting that this devfile is incompatible with 7.0.0 I believe, since it depends on eclipse-che/che#14174 cc: @ibuziuk

@amisevsk
Copy link
Contributor

Logs from the failing container


Starting the deployer with the list of resolvers [ LocalDirectoryPluginDeployerResolver {},
--
  | GithubPluginDeployerResolver { unpackedFolder: '/tmp/github-remote' },
  | HttpPluginDeployerResolver { unpackedFolder: '/tmp/http-remote' },
  | VsCodePluginDeployerResolver { vscodeExtensionsFolder: '/tmp/vscode-extension-marketplace' } ]
  | Theia Endpoint 19/pid listening on port 2503
  | Found the list of default plugins ID on env: undefined
  | Found the list of plugins ID on env: local-dir:///plugins/sidecars/che_incubator_cpptools_latest
  | Found the list of default plugins ID from CLI: undefined
  | unzipping the plugin ProxyPluginDeployerEntry {
  | deployer:
  | PluginVsCodeFileHandler { unpackedFolder: '/tmp/vscode-unpacked' },
  | delegate:
  | PluginDeployerEntryImpl {
  | originId:
  | 'local-dir:///plugins/sidecars/che_incubator_cpptools_latest',
  | pluginId:
  | 'che-incubator.cpptools.latest.hnoaqagcwz.cdt-vscode-0.0.7.vsix',
  | map: Map {},
  | changes: [],
  | acceptedTypes: [],
  | currentPath:
  | '/plugins/sidecars/che_incubator_cpptools_latest/che-incubator.cpptools.latest.hnoaqagcwz.cdt-vscode-0.0.7.vsix',
  | initPath:
  | '/plugins/sidecars/che_incubator_cpptools_latest/che-incubator.cpptools.latest.hnoaqagcwz.cdt-vscode-0.0.7.vsix',
  | resolved: true,
  | resolvedByName: 'LocalDirectoryPluginDeployerResolver' },
  | deployerName: 'PluginVsCodeFileHandler' }
  | unzipping the plugin ProxyPluginDeployerEntry {
  | deployer:
  | PluginVsCodeFileHandler { unpackedFolder: '/tmp/vscode-unpacked' },
  | delegate:
  | PluginDeployerEntryImpl {
  | originId:
  | 'local-dir:///plugins/sidecars/che_incubator_cpptools_latest',
  | pluginId:
  | 'che-incubator.cpptools.latest.hwcyhssqiw.llvm-vs-code-extensions.vscode-clangd-0.0.16.vsix',
  | map: Map {},
  | changes: [],
  | acceptedTypes: [],
  | currentPath:
  | '/plugins/sidecars/che_incubator_cpptools_latest/che-incubator.cpptools.latest.hwcyhssqiw.llvm-vs-code-extensions.vscode-clangd-0.0.16.vsix',
  | initPath:
  | '/plugins/sidecars/che_incubator_cpptools_latest/che-incubator.cpptools.latest.hwcyhssqiw.llvm-vs-code-extensions.vscode-clangd-0.0.16.vsix',
  | resolved: true,
  | resolvedByName: 'LocalDirectoryPluginDeployerResolver' },
  | deployerName: 'PluginVsCodeFileHandler' }
  | unzipping the plugin ProxyPluginDeployerEntry {
  | deployer:
  | PluginVsCodeFileHandler { unpackedFolder: '/tmp/vscode-unpacked' },
  | delegate:
  | PluginDeployerEntryImpl {
  | originId:
  | 'local-dir:///plugins/sidecars/che_incubator_cpptools_latest',
  | pluginId:
  | 'che-incubator.cpptools.latest.hzgonxjjgp.cdt-gdb-vscode-0.0.90.vsix',
  | map: Map {},
  | changes: [],
  | acceptedTypes: [],
  | currentPath:
  | '/plugins/sidecars/che_incubator_cpptools_latest/che-incubator.cpptools.latest.hzgonxjjgp.cdt-gdb-vscode-0.0.90.vsix',
  | initPath:
  | '/plugins/sidecars/che_incubator_cpptools_latest/che-incubator.cpptools.latest.hzgonxjjgp.cdt-gdb-vscode-0.0.90.vsix',
  | resolved: true,
  | resolvedByName: 'LocalDirectoryPluginDeployerResolver' },
  | deployerName: 'PluginVsCodeFileHandler' }
  | PluginTheiaDirectoryHandler: accepting plugin with path /tmp/vscode-unpacked/che-incubator.cpptools.latest.hnoaqagcwz.cdt-vscode-0.0.7.vsix
  | Resolving "che-incubator.cpptools.latest.hnoaqagcwz.cdt-vscode-0.0.7.vsix" as a VS Code extension...
  | Resolved "che-incubator.cpptools.latest.hnoaqagcwz.cdt-vscode-0.0.7.vsix" to a VS Code extension "cdt-vscode@0.0.7" with engines: { vscode: '^1.30.0' }
  | PluginTheiaDirectoryHandler: accepting plugin with path /tmp/vscode-unpacked/che-incubator.cpptools.latest.hwcyhssqiw.llvm-vs-code-extensions.vscode-clangd-0.0.16.vsix
  | Resolving "che-incubator.cpptools.latest.hwcyhssqiw.llvm-vs-code-extensions.vscode-clangd-0.0.16.vsix" as a VS Code extension...
  | Resolved "che-incubator.cpptools.latest.hwcyhssqiw.llvm-vs-code-extensions.vscode-clangd-0.0.16.vsix" to a VS Code extension "vscode-clangd@0.0.16" with engines: { vscode: '^1.36.0' }
  | PluginTheiaDirectoryHandler: accepting plugin with path /tmp/vscode-unpacked/che-incubator.cpptools.latest.hzgonxjjgp.cdt-gdb-vscode-0.0.90.vsix
  | Resolving "che-incubator.cpptools.latest.hzgonxjjgp.cdt-gdb-vscode-0.0.90.vsix" as a VS Code extension...
  | Resolved "che-incubator.cpptools.latest.hzgonxjjgp.cdt-gdb-vscode-0.0.90.vsix" to a VS Code extension "cdt-gdb-vscode@0.0.90" with engines: { vscode: '^1.26.0' }
  | /home/theia/node_modules/@theia/plugin-ext-vscode/lib/node/plugin-vscode-resolver.js:129
  | asset = extension.versions[0].files.filter(function (f) { return f.assetType === 'Microsoft.VisualStudio.Services.VSIXPackage'; })[0];
  | ^
  |  
  | TypeError: Cannot read property 'versions' of undefined
  | at Request._callback (/home/theia/node_modules/@theia/plugin-ext-vscode/lib/node/plugin-vscode-resolver.js:129:55)
  | at Request.self.callback (/home/theia/node_modules/request/request.js:185:22)
  | at Request.emit (events.js:198:13)
  | at Request.<anonymous> (/home/theia/node_modules/request/request.js:1161:10)
  | at Request.emit (events.js:198:13)
  | at IncomingMessage.<anonymous> (/home/theia/node_modules/request/request.js:1083:12)
  | at Object.onceWrapper (events.js:286:20)
  | at IncomingMessage.emit (events.js:203:15)
  | at endReadableNT (_stream_readable.js:1145:12)
  | at process._tickCallback (internal/process/next_tick.js:63:19)


@tolusha
Copy link
Contributor Author

tolusha commented Aug 27, 2019

@amisevsk
Thank you for validation

eclipse-theia/theia#5993

@tsmaeder tsmaeder mentioned this pull request Aug 29, 2019
41 tasks
Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested again on current nightlies:

The workspace starts up no problem and I can execute build.

However, I get an error after opening the .cpp file:

Error starting C/C++ language server. Please make sure 'clangd' is installed on your system. You can refer to the clangd page for instructions.

Additionally, I'm against merging this PR using a community image (registry.centos.org/centos/devtoolset-7-toolchain-centos7) as it has most of the problems the arbitrary user id patch was made to resolve. The steps to adding a patched image are

  1. Add a line like che-cpp-centos registry.centos.org/centos/devtoolset-7-toolchain-centos7 to base_images
  2. Test that builds okay ./arbitrary-users-patch/build_images.sh
  3. Create quay.io repo for the new image https://quay.io/organization/eclipse/
  4. Update devfile to use this new image.

devfiles/cpp/devfile.yaml Show resolved Hide resolved
devfiles/cpp/devfile.yaml Outdated Show resolved Hide resolved
@tolusha
Copy link
Contributor Author

tolusha commented Sep 12, 2019

Remaining issue:

Error starting C/C++ language server. Please make sure 'clangd' is installed on your system. You can refer to the clangd page for instructions.

devfiles/cpp/devfile.yaml Outdated Show resolved Hide resolved
@nickboldt nickboldt self-requested a review September 23, 2019 16:46
Copy link
Contributor

@nickboldt nickboldt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per @amisevsk 's comment #73 (review) I've created this PR to add the cpp stack:

#100

Copy link
Contributor

@nickboldt nickboldt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per @amisevsk 's comment #73 (review) I've created this PR to add the cpp stack:

#100

Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
@tolusha
Copy link
Contributor Author

tolusha commented Sep 25, 2019

@nickboldt
I see you've added
che-cpp-rhel7 registry.access.redhat.com/devtools/llvm-toolset-rhel7

Is it supposed to be used in c++ stack instead of che-cpp7-centos ?
What are the benefits?

@nickboldt
Copy link
Contributor

Is it supposed to be used in c++ stack instead of che-cpp7-centos ?

Yes

What are the benefits?

  1. same upstream and downstream, no need to fork dockerfile for community/centos and product/rhel: less work in downstream

  2. same bits, effectively, since no one will be trying to update the container and need to worry about subscriptions/entitlements (which is the real value for centos over rhel, but now that we have UBI and it's just as free as centos, there's no need to use centos): we use the free supported RHEL instead of the free community-built RHEL

  3. CVE fixes come faster in UBI than in CentOS.

-
type: dockerimage
alias: cpp-dev
image: quay.io/eclipse/che-cpp7-centos:nightly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switch to use che-cpp-rhel7

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this image available publicly?

Copy link
Contributor Author

@tolusha tolusha Sep 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be available once I merge the PR
It is available

@amisevsk
Copy link
Contributor

Is it supposed to be used in c++ stack instead of che-cpp7-centos ?

This new image is meant to resolve #73 (comment) (it includes clang). RHEL was chosen because it's @nickboldt's PR ;)

I've tested the devfile in this PR, updating image to che-cpp-rhel7 and everything seems to work.

devfiles/cpp/devfile.yaml Outdated Show resolved Hide resolved
Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
@tolusha
Copy link
Contributor Author

tolusha commented Sep 26, 2019

I've fixed remarks and tested.
Everything works well.
@amisevsk @nickboldt
Could you approve the PR?

Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @tolusha

Copy link
Contributor

@nickboldt nickboldt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@nickboldt nickboldt merged commit 3caaf7c into master Sep 26, 2019
@nickboldt
Copy link
Contributor

squashed and merged.

@nickboldt nickboldt deleted the ab/cpp branch February 12, 2020 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants