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

Create devcontainer.json #13520

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Create devcontainer.json #13520

wants to merge 1 commit into from

Conversation

bam80
Copy link
Contributor

@bam80 bam80 commented Sep 23, 2023

Maintainer: me
Compile tested: mips, ath79, OpenWrt SNAPSHOT

Description:
Development container configuration.
The container can run locally or in cloud (Codespaces).
Sample usage: easily open full-featured VS Code IDE in browser, pre-configured for OpenWrt firmware/packages development.

Signed-off-by: Andrey Butirsky butirsky@gmail.com

image

@john-tho
Copy link
Contributor

in cloud (Codespaces)
in browser

Could you please give examples of what benefits this might provide? Which target SDK does this use? How does it help users do runtime testing of the changes they are proposing?

If this encourages more users to submit PRs via GitHub's web development tools, could we please hold off until GitHub can work with a rebase workflow? Advising users to squash, reword, or not to spam PRs to update is not productive for anyone, this week's example: #13486 (comment)

@bam80
Copy link
Contributor Author

bam80 commented Oct 2, 2023

Could you please give examples of what benefits this might provide?

For example, it allows one who has no local dev setup or doesn't want to checkout code locally still participate in the OpenWrt/packages development.
It also provides better code navigation, as VS Code does it better than Github.

Which target SDK does this use?

The SDK is generated from this repository:
https://github.com/openwrt/docker#sdk
You can see possible targets here:
https://hub.docker.com/r/openwrt/sdk/tags

In my test, by default it compiled a package for mips, ath79.

How does it help users do runtime testing of the changes they are proposing?

Users can download the compiled images/packages from the container locally for the tests.

If this encourages more users to submit PRs via GitHub's web development tools, could we please hold off until GitHub can work with a rebase workflow?

I don't think this encourages more GitHub's web development tools usage.
Rather makes the Green Button do the right thing, as it exists anyway.
The container has full CLI access to the official OpenWrt SDK, which includes mainstream git tools to make a PRs.

So I would say this doesn't harm anything, but can be extremely useful.
Probably no reasons to block it.

Copy link
Member

@ynezz ynezz left a comment

Choose a reason for hiding this comment

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

LGTM. Unless someone beats me to it, I'll leave it here for next two weeks so it gets a chance for review and then merge this myself.

@bam80
Copy link
Contributor Author

bam80 commented Oct 18, 2023

@ynezz ping please?

@hauke
Copy link
Member

hauke commented Oct 29, 2023

Why do you use the OpenWrt SDK container?
When I use the default code space environment I am able to build OpenWrt inside the system.

I added a codespace container to build OpenWrt using the SDK: https://github.com/hauke/openwrt/blob/codespace/.devcontainer/ci-env/devcontainer.json
This helped me some time ago to debug build problems seen only in the SDK.

Maintainer: me
Compile tested: mips, ath79, OpenWrt SNAPSHOT

Description:
Development container configuration.
The container can run locally or in cloud (Codespaces).
Sample usage: easily open full-featured VS Code IDE in browser,
pre-configured for OpenWrt firmware/packages development.

Signed-off-by: Andrey Butirsky <butirsky@gmail.com>
@bam80
Copy link
Contributor Author

bam80 commented Oct 30, 2023

Why do you use the OpenWrt SDK container?
When I use the default code space environment I am able to build OpenWrt inside the system.

Can you cross-compile with the default code space environment, out of the box?
While the default code space environment might work for you at the moment, it can change at any time, whereas OpenWrt SDK is the official reference thing for OpenWrt, so we are better stick to it.

@ynezz
Copy link
Member

ynezz commented Oct 30, 2023

Can you cross-compile with the default code space environment, out of the box?

IMO (and if its possible) we should allow both use cases. Any idea how to achieve that? Maybe we should consider ImageBuilder containers in the mix? So we could've codespace for system build, package build with SDK and image builder.

Which target SDK does this use?

Good point, how to handle this? Is it possible to have some kind of a list for each platform we support?

@bam80
Copy link
Contributor Author

bam80 commented Oct 30, 2023

@ynezz right now, if we merge both this and #13825, the list of the both container configurations should be presented (but I didn't test it).

The same way we could handle all the others platforms, making separate devcontainer.json for each.
From there, we could reference different platforms SDKs by tag:
https://hub.docker.com/r/openwrt/sdk/tags

I'm not sure how maintainable that galore of devcontainer.json would be, though.

@ynezz
Copy link
Member

ynezz commented Oct 30, 2023

the list of the both container configurations should be presented (but I didn't test it).

Seems to work fine, even with all SDK containers.

image

I'm not sure how maintainable that galore of devcontainer.json would be, though.

We could add script to generate those ynezz@9d22687

@bam80
Copy link
Contributor Author

bam80 commented Oct 30, 2023

Seems to work fine, even with all SDK containers.

It probably has no point to generate the .json's for all SDK versions in the master branch.
Those for other versions could go to respective branches, if we need them at all.

@bam80
Copy link
Contributor Author

bam80 commented Oct 30, 2023

We could add script to generate those ynezz@9d22687

It looks like Dev Container Templates provide better answer for this, see

     "image": "mcr.microsoft.com/devcontainers/java:0-${templateOption:imageVariant}",

in the .devcontainer.json example:
https://containers.dev/implementors/templates/#option-resolution-example

@ynezz could we have a similar script but to generate imageVariant, etc. template options for devcontainer-template.json instead?
See the example above.

The options would be
target, subtarget, branch|tag|version as per:
https://github.com/openwrt/docker#sdk-tags

Having that, here is a guide for creating our own custom Dev Container Templates:
https://github.com/devcontainers/template-starter

UPDATE:
the script should probably be on the OpenWrt docker repo side, somewhere here:
https://github.com/openwrt/docker/blob/main/.github/workflows/containers.yml#L150

I also created a relevant issue there:
openwrt/docker#126

@hauke
Copy link
Member

hauke commented Oct 30, 2023

having all SDKs available here would be nice. I agree we should only show the master SDKs in master and add the others to the other branches.

The template looks very nice. Does the template work with github? The github devcontainer documentation does not mention it.

@bam80
Copy link
Contributor Author

bam80 commented Oct 31, 2023

having all SDKs available here would be nice.

Addressing it on OpenWrt docker repo side should guarantee this and I think overall better as there is already all the Tags parsing needed there:
openwrt/docker#126

The template looks very nice. Does the template work with github? The github devcontainer documentation does not mention it.

Looks like it does. Worst thing I can imagine the template won't be applied for our repo automagically, and user has to select the OpenWrt SDK Template from a filterable list displayed on the GitHub Codespaces or on the VS Code Dev Containers UI:
https://code.visualstudio.com/docs/devcontainers/containers#_quick-start-open-an-existing-folder-in-a-container

But that needs to be tested, it might be there is some logic to display most appropriate templates for the repo on the top of the list, or something like that.

Anyway, it seems the way to go forward.

@bam80
Copy link
Contributor Author

bam80 commented May 22, 2024

@ynezz right now, if we merge both this and #13825, the list of the both container configurations should be presented

Friendly ping.
The patch suggested is useful as is, and it's simple.
Could we merge it to have something, and then iterate from that, please?

@bam80
Copy link
Contributor Author

bam80 commented Jun 16, 2024

Ping, please

@bam80
Copy link
Contributor Author

bam80 commented Jun 26, 2024

@hauke ping, please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants