I'm not sure how batch files work, but for bash maybe we should factor out the command logic between this and //bin/flutter into a third shell script that these two source?

I generally don't like sourcing outside scripts in shell scripts, but I feel like these dart scripts are very likely to drift in logic from their flutter counterparts.

Another option is to have both //bin/flutter and //bin/dart just be wrappers that invoke the third script, and then the common script has branching logic depending on which entrypoint it was invoked from.

For the bash changes I looked at https://stackoverflow.com/questions/192292/how-best-to-include-other-scripts which scared me quite a bit.

I'm not really that familiar with shell scripting. Maybe we can chat a bit tomorrow to figure out how that would work?

I agree less duplicate is better.

Yeah, let's chat tomorrow. And you're right to be scared :P

I did a little work on flutter's shell script here and @devoncarew asked me to poke my head in. I won't say some caution isn't warranted but a few basic best practices (adapted for Flutter's situation) should keep you from accidentally making your shell scripts any more unreadable than strictly necessary.

1. Determine FLUTTER_ROOT first, then use that to calculate the path of any file you source. Those suggestions in the stackoverflow are kind of ad-hoc and terrifying... I can suggest some alternative reading material offline if you are interested.
2. No file you source should do anything but declare functions -- if you want to share common variable initialization, move those to functions. In fact, do as little as you can outside of functions generally -- it makes the script much easier to follow. Put things you would ordinarily have in the top level in a main function.
3. Use local extensively to declare variables and minimize your use of globals where you can. Globals in CAPS_WITH_UNDERSCORES, locals with lower_case_underscored. The script mostly does this already...
4. Decide on a naming convention for your functions that make sense and minimize namespace collisions. For example: function flutter::common::retry_upgrade () {} for a function defined in the library 'common.sh'. If you use that style consistently it will scale up as far as you need and also makes it more clear when you're calling something local vs. sourced.

Like any set of best practices you can break them if you know what you're doing, but I humbly suggest starting with these in your implementation.

As I've been thinking about this, I can think of 3 options to keep us from duplicating code:

1. Move shared code into functions in a seperate script and source them from the flutter and dart entrypoints, ala @jcollins-g's comment above.
2. Move all the logic currently in //bin/flutter into //bin/launcher.sh. Create two symlinks to that script at //bin/flutter and //bin/dart. Then in //bin/launcher.sh, read which entrypoint the user used ENTRYPOINT="$(basename "$BASH_SOURCE")" and then add branching logic to the script based on the value of $ENTRYPOINT where needed.
3. Similar to the last method, but instead of using symlinks, just have short wrapper shell scripts, and detect our entrypoint via environment variable. Proof of concept here.

Problems with these:

1. Sourcing shell scripts are tricky and easy to get wrong.
2. I noticed we don't use symlinks anywhere in our repo. Not sure if these links could get broken by one of the release channels.
3. Additional overhead of one more wrapper script.

@gspencergoog WDYT?

Symlinks scare me. Option number 2 works fine until someone makes their own symlink to point to them (either to the symlink or the launcher.sh). I think I'd rather we went with option number 1, even given the pitfalls. Option 3 could also work, but I don't think that's much different from option 1, really. You're exec'ing instead of sourcing, but the problems with safely locating the script to invoke are the same.

I'd like to reiterate the principle of putting as little logic as possible into these wrappers, however. We should start Dart and use that for anything complex as soon as we've bootstrapped far enough to be able to do that.

BTW, I endorse all the things @jcollins-g said above.

Makes sense, sounds like we should go with option 1, following @jcollins-g's guidelines.

I'm thinking we should also setup CI to shellcheck all our shell scripts.

we don't need $FLUTTER_TOOL_ARGS (unless this ever gets set externally?)

I'm not sure how to handle this, but I expect users are going to be confused why invoking dart is re-building their flutter tool.

If we exit early here, without updating $STAMP_PATH, the next time the user invokes the flutter tool it will rebuild. Although it means disrupting the user's flow twice, I think at least that flow is more expected. WDYT?

I don't think we should rebuild their flutter tool unless they're actually using the flutter tool.

Why do you say this disrupts them twice? Isn't it just the once when they try to invoke the flutter tool (as it is today)? I mean, sure, it invalidates the cache earlier this way, but I don't think the user will notice that until they run flutter, and the flutter tool is going to rebuild at most once for each cache invalidation anyhow. At least this will have updated the dart SDK earlier for them.

Plus, if they don't need to run the flutter tool, then we're wasting their time/energy/battery rebuilding it.

keep in mind that as contributors to flutter you will hit this code path approximately one million times more often than someone who upgrades stable -> stable.

Let's keep it simple and reduce conditional logic to the minimum.

TYPO: hide -> hit