vscode chokes on mismatched submodules

[hamishwillee]

I had a built version of Firmware master and switched to a branch (this one but I doubt it matters). Because the submodules are out of sync the IDE stalled during configuring as shown below.

Ideally the system should be smart enough to help you out, or at least let you know configuring will never end.

[dagar]

This one might be a bit tricky. We can be a little more heavy handed and update things automatically, but that runs the risk of someone losing partial changes in a submodule they wanted to keep. On the other hand it's better for the vast majority of users the vast majority of the time...

[hamishwillee]

Perhaps just leave as is and solve in documentation.

[julianoes]

My opinion is that we should remove this submodule check. We should educate people about submodules using documentation and remove magic like that. I know not many agree with me on that but it would prevent issues like that. I myself have been using GIT_SUBMODULES_ARE_EVIL=1 for years, I really hate the automated checks!

[LorenzMeier]

Please educate people about the option (better docs), but we will leave the hard check in. We had dozens of support requests (and badly built software that failed) because people generally do not understand it.

We also had people that were flying unsafe software because they were missing fixes in e.g. ECL. Maintainers like yourself are a special breed of developers and we shouldn't build our dev workflow to suite them. You can already disable it and that is what everybody else should be doing.

[julianoes]

I understand where we're coming from but I also think you should not be compiling and flying stuff if you don't know how to read git status.

[LorenzMeier]

We can continue that argument - we could also argue that you should not be flying something if you don't know how to tune a system completely from scratch - so let's set all gains to zero and provide a tuning guide.

Where that leads us that you need to be a master of everything to work with PX4, which limits the audience to a handful of people. That world view is not the DNA of our project and would dramatically hinder its long-term sustainability (it led to the demise of BSD over Linux, as the BSD folks were extremely opinionated about which skills you need to have - it was arguably probably the better built system, but that doesn't help if you have significantly less adoption). We have to live with the fact that our user base is quite diverse, ranging from electrical engineers to mechanical engineers to computer scientists to students who don't know any of the above disciplines and our job is to make every angle of the system as approachable as possible.

And once people are able to work with it, they can enable expert mode for the part they are interested in - like re-tuning controller gains or disabling submodules.

[julianoes]

You can also argue that it is elitist to assume that you don't want to teach the developers the skills necessary to work with the code base and instead simplify or hide it.

I don't necessarily agree with the analogy because I am talking about the general tooling skills that a developer/engineer/programmer should learn and not the knowledge about how to use a specific aspect of the PX4 software.

Anyway, I understand the checks are here to stay unless we figure out something easier than submodules.

[dagar]

My high level goal for PX4 is to try and make the bulk of the typical workflow easy and approachable (somewhat like a commercial IDE), but without preventing advanced users from jumping in and doing what they want.

For example we have submodule hooks and checks builtin, but you can skip them with an environment variable. There's a top level Makefile that wraps the typical cmake usage, but you can bypass it and work directly. We have an opinionated VSCode project in tree, but it goes out of its way to place artifacts compatibly so you can move back and forth between IDE and CLI. These things might seem trivial now, but they all required extra deliberate work.

The submodule case could still be better, but it's less destructive than it was historically and easy enough to continue using even when developing within submodules. @julianoes I would encourage you to give it another try (at least briefly) and if you hit something annoying let's consider if it's possible to fix or compare alternatives. I've spent enough time trying to help people get through typical submodule usage that I'm still confident these hooks are better than the alternative.

[hamishwillee]

@dagar agree 100% with make the bulk of the typical workflow easy and approachable ... but without preventing advanced users from jumping in and doing what they want.

We might argue whether the line is drawn correctly though. Consider:

1. The instructions are pitched for ONLY FOR EXPERTS but if you're not an expert it isn't clear what you should do
2. I always have to Ctrl+C out of this and do git submodule update because I'm not playing with submodules. Personally I'd prefer that offered a prompt first like

   Some git submodules are not in recommended versions. Enter Y to reset them or N to resolve individually?"

Does VSCode allow you to trigger a UI prompt?

As an aside, how do those expert users find out about the environment variables?

[dagar]

Let me see what else we can do here (at least within VSCode). Obviously we still have quite a lot to improve towards that high level goal.

[LorenzMeier]

The instructions for normal users are above Hamish. So the action item would be to shorten them.

[hamishwillee]

@LorenzMeier Thanks, I feel a bit silly missing that.

IMO all the information presented is useful, so a better solution (if possible) to offer the beginner/reset all prompt first and separately. If they don't choose to accept then they do the expert option.

This makes the decision at each point binary, and obvious.

[dagar]

A few improvements are coming together here. https://github.com/PX4/Firmware/pull/13067

[dagar]

This is also worth trying. https://marketplace.visualstudio.com/items?itemName=ivanhofer.git-assistant

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

[julianoes]

Is this still an issue?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.