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

[docs] Add section 'Dev Container' to the Mojo stdlib docs #2664

Closed
wants to merge 3 commits into from

Conversation

benz0li
Copy link
Contributor

@benz0li benz0li commented May 15, 2024

Test online with GitHub Codespaces: https://codespaces.new/benz0li/mojo-dev-container?hide_repo_select=true&ref=main

Parent image: glcr.b-data.ch/mojo/base:nightly
👉 This image is being rebuilt at the start of every 4th hour.

@benz0li benz0li force-pushed the add-dev-container branch 2 times, most recently from faf1c07 to 9ce40a5 Compare May 15, 2024 15:32
@benz0li benz0li changed the title Add Dev Container Configuration Files [tools] Add Dev Container Configuration Files May 15, 2024
@benz0li benz0li force-pushed the add-dev-container branch from 9ce40a5 to 907b5c0 Compare May 15, 2024 15:56
@benz0li
Copy link
Contributor Author

benz0li commented May 15, 2024

Cross reference:

@JoeLoser
Copy link
Collaborator

Thanks for the contribution! Can you help me understand some of the rationale on why we'd want to take this change into this repo rather than suggesting you (or someone else) maintain these sort of dev containers from a given published nightly mojo release? Does it help core contributors to the standard library in some way, or mostly just users of Mojo (not stdlib contributors)? @gabrieldemarmiesse do you have any thoughts here?

@JoeLoser JoeLoser requested review from a team and zbowling May 22, 2024 18:17
@benz0li
Copy link
Contributor Author

benz0li commented May 22, 2024

Thanks for the contribution! Can you help me understand some of the rationale on why we'd want to take this change into this repo rather than suggesting you (or someone else) maintain these sort of dev containers from a given published nightly mojo release?

Already there. Multi-arch (linux/amd64, linux/arm64/v8) Mojo Dev Containers: https://github.com/b-data/data-science-devcontainers

  • Mojo base (nightly)
  • Mojo base
  • Mojo scipy (nightly)
  • Mojo scipy

Open in GitHub Codespaces

Parent images: Mojo docker stack (Debian-based)

@benz0li
Copy link
Contributor Author

benz0li commented May 22, 2024

Does it help core contributors to the standard library in some way, or mostly just users of Mojo (not stdlib contributors)?

For stdlib contributors: The Mojo (nightly) Dev Container provides a unified development environment.
ℹ️ Currently, llvm-14 and llvm-14-tools are installed in the Dev Containers.
👉 Better v17 with https://apt.llvm.org/llvm.sh as in .github/workflows/examples.yml?

For Mojo users: In case of an issue, the Mojo Dev Container serves as a reference [Linux] deployment for reproduction.
ℹ️ No more "it works on my machine"-discussions.

@benz0li
Copy link
Contributor Author

benz0li commented May 22, 2024

The Data Science Dev Containers are meant for Data Scientists, Mojo users and alike.

The Dev Containers proposed here are for core contributors to the standard library.

And with GitHub Codespaces, anyone can set up a Mojo development environment.

@benz0li benz0li marked this pull request as draft May 22, 2024 19:03
@gabrieldemarmiesse
Copy link
Contributor

@benz0li thank you very much for creating the dev container! @JoeLoser I think this is a must-have in the future for users and for our contributors' experience.

While it's extremely easy currently to install the nightly version of Mojo, it might not be as easy to install all the tools needed to run the unit tests (llvm as @benz0li said, or other) and this provides a codespace environment where users can work with Mojo in their browser. That is useful for users who are on platforms not supported by the Mojo sdk or users who want to try Mojo at work but have very tight restrictions on what they can install on their machine. It's also nice as codespace allows you easier reviewing of PRs and quick code modifications without firing your own IDE, adding the remote, checking out the branch, etc...

@benz0li The issue we have right now with the Mojo repository is that the maintainers (specifically, the ones with merging rights) are understaffed. As you can see, a lot of PRs are open, and we have to be very selective with what kind of pull request we accept. We already delayed multiple propositions for new structs and modules, and in my opinion, we should be even more selective with what PRs to accept (we should create a list of new functions that are wanted right now and everything outside of this should be rejected/postponed until we make more progress).

Every piece of code getting into the nightly branch becomes the responsability of the maintainers. Thus if we merge this right now, the maintainers give the garantee to the contributors that the workflow enabled by those 400 lines of code will work all the time, and through updates. We sadly don't have much time to spare for the maintenance. We would need to keep up to date the base image, the llvm install, the list of vscode extensions, etc... Debugging this can take a while too in case of bugs.

My advice would be: Since this is very useful, let's let the contributors know about it through a section in the contributing guide, saying "hey, if you have trouble setting up your environment, just go to https://github.com/b-data/data-science-devcontainers and there are dev containers with everything already installed, and it works with github codespace!". Since this is not our repository, we give no garantee that it works or will be maintained, and for every bug, users will create an issue in https://github.com/b-data/data-science-devcontainers and not in https://github.com/modularml/mojo . When we have more maintainers, and that we have enough bandwidth to maintain a dev workflow in a dev container, then we will bring this PR back to life. Especially since, in the future, I believe it will become easier to install the tools necessary. For example, we'll soon use the Mojo testing framework and I suppose llvm won't be needed! This will simplify the dockerfile :)

@benz0li
Copy link
Contributor Author

benz0li commented May 23, 2024

My advice would be: Since this is very useful, let's let the contributors know about it through a section in the contributing guide, saying "hey, if you have trouble setting up your environment, just go to https://github.com/b-data/data-science-devcontainers and there are dev containers with everything already installed, and it works with github codespace!".

I am going to set up a dedicated Mojo Dev Container, then.

  1. Including LLVM v17 (installed using https://apt.llvm.org/llvm.sh)
  2. The repository checked out at ~/projects/modularml/mojo
  3. The user's working directory set to the repository
  4. Branch nightly already checked out

ℹ️ You will be notified here when it is ready.

Since this is not our repository, we give no garantee that it works or will be maintained, and for every bug, users will create an issue in https://github.com/b-data/data-science-devcontainers and not in https://github.com/modularml/mojo .

I am happy to maintain.

@gabrieldemarmiesse
Copy link
Contributor

gabrieldemarmiesse commented May 24, 2024

Great, feel free to modify this PR to add a section to the contributing guide so that new contributors know they can go to Mojo Dev Containers to setup their dev container :)

@benz0li
Copy link
Contributor Author

benz0li commented May 24, 2024

Great, feel free to modify this PR to add a section to the contributing guide so that new contributors know they can go to Mojo Dev Containers to setup their dev container :)

The Mojo Dev Container is available now. Please be aware of benz0li/mojo-dev-container#1

I will modify this PR, i.e. add a section to the contributing guide pointing to the Mojo Dev Container.

@gabrieldemarmiesse
Copy link
Contributor

Please be aware of benz0li/mojo-dev-container#1

strange, but it's a good step forward :)

Thanks for your awesome work, that will hopefully bring Mojo to more users!

@benz0li benz0li force-pushed the add-dev-container branch from 907b5c0 to 86ba12f Compare May 24, 2024 12:50
@benz0li benz0li changed the title [tools] Add Dev Container Configuration Files [docs] Add section 'Dev Container' to the Mojo stdlib docs May 24, 2024
Signed-off-by: Olivier Benz <olivier.benz@b-data.ch>
@benz0li benz0li force-pushed the add-dev-container branch from 86ba12f to 388e173 Compare May 24, 2024 12:51
@benz0li benz0li marked this pull request as ready for review May 24, 2024 12:59
@benz0li benz0li requested a review from a team as a code owner May 24, 2024 12:59
@JoeLoser JoeLoser self-assigned this May 31, 2024
.devcontainer/Mojo.Dockerfile Outdated Show resolved Hide resolved
stdlib/docs/development.md Outdated Show resolved Hide resolved
Signed-off-by: Joe Loser <joeloser@fastmail.com>
Signed-off-by: Joe Loser <joeloser@fastmail.com>
Copy link
Collaborator

@JoeLoser JoeLoser left a comment

Choose a reason for hiding this comment

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

Thanks for writing this up! Happy to see this get incorporated. I'll check out #2852 next week as well.

@JoeLoser
Copy link
Collaborator

!sync

@modularbot modularbot added the imported-internally Signals that a given pull request has been imported internally. label May 31, 2024
@modularbot
Copy link
Collaborator

✅🟣 This contribution has been merged 🟣✅

Your pull request has been merged to the internal upstream Mojo sources. It will be reflected here in the Mojo repository on the nightly branch during the next Mojo nightly release, typically within the next 24-48 hours.

We use Copybara to merge external contributions, click here to learn more.

@modularbot modularbot added the merged-internally Indicates that this pull request has been merged internally label May 31, 2024
@modularbot
Copy link
Collaborator

Landed in 03e215e! Thank you for your contribution 🎉

@modularbot modularbot added the merged-externally Merged externally in public mojo repo label Jun 1, 2024
modularbot pushed a commit that referenced this pull request Jun 1, 2024
… (#41065)

[External] [docs] Add section 'Dev Container' to the Mojo stdlib docs

Test online with GitHub Codespaces:
https://codespaces.new/benz0li/mojo-dev-container?hide_repo_select=true&ref=main

Parent image: `glcr.b-data.ch/mojo/base:nightly`
👉 This image is being rebuilt [at the start of every 4th
hour](https://cron.help/every-4-hours).

Co-authored-by: Olivier Benz <olivier.benz@b-data.ch>
Closes #2664
MODULAR_ORIG_COMMIT_REV_ID: 96e7a1d0b243761a48bd4af3bd1f5f7441abcc85
@modularbot modularbot closed this Jun 1, 2024
modularbot pushed a commit that referenced this pull request Sep 13, 2024
… (#41065)

[External] [docs] Add section 'Dev Container' to the Mojo stdlib docs

Test online with GitHub Codespaces:
https://codespaces.new/benz0li/mojo-dev-container?hide_repo_select=true&ref=main

Parent image: `glcr.b-data.ch/mojo/base:nightly`
👉 This image is being rebuilt [at the start of every 4th
hour](https://cron.help/every-4-hours).

Co-authored-by: Olivier Benz <olivier.benz@b-data.ch>
Closes #2664
MODULAR_ORIG_COMMIT_REV_ID: 96e7a1d0b243761a48bd4af3bd1f5f7441abcc85
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
imported-internally Signals that a given pull request has been imported internally. merged-externally Merged externally in public mojo repo merged-internally Indicates that this pull request has been merged internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants