Why not just leave this as a fallback for if it's not set, rather than making it a required parameter?

Removing this may break non-flutter run cases; we should probably leave it until the copy is being done from the Flutter side.

The makefile doesn't read FLUTTER_ENGINE_VERSION_FILE, it's just part of the timestamp analysis process. Your other patch writes this file unconditionally, so regardless of what's being written into it, the rules depending on this file are always going to re-run.

Add missing newline please (to avoid churn next time I edit it, since my editors are generally configured to add them)

^^^

This should just be removed, along with the dependency below using it; it no longer makes sense to have the Makefile have a concept of when the copies need to be updated since it doesn't have a way to copy them.

Although now that I'm thinking about it that way, there's a structural problem with this pair of patches: as written, make will no longer work reliably, because the copies of the files won't be updated. We need to follow the macOS model where there's a script that we can call to do copying and such but that doesn't invoke make, so that make can call it. Sorry I missed this when reviewing the other patch.

The tool re-entrance requirements add a lot of complexity to this process - It only kind of works at best too.

It is a requirement for iOS due to Xcode and for Android due to Gradle, but I don't see a compelling case yet for Windows or Linux.

It is a requirement for iOS due to Xcode and for Android due to Gradle, but I don't see a compelling case yet for Windows or Linux.

I'm not sure I follow. How is Xcode fundamentally different from Visual Studio, or Make (or whatever other "native" build system we settle on for Linux)? In all these cases, the use case seems to be the same: "I want to build with a native workflow I'm used to for my platform"

These workflows do not work if you don't run flutter first - making the tooling call to copy the assets over re-entrant does not change this

Only because we don't have a flutter create pre-populating some of the generated files, right?

There's a difference between needing to have run the flutter tool once in the past in order to do a build with the underlying build system, and not being able to do a build with the underlying build system in the future either.

And I'm still not clear on how Xcode is different here.

What is the workflow we are designing for here? Is this for changing local engine sources, or for working with the native application code?

What I mean is, lets go back about 6 steps:

1. The makefile copies the required sources into its own cache directory.
2. The tool can write a config flag which overwrites the default for local engine builds.
3. ???
   4 Profit

I don't want to design a build system for one workflow though; we should have a build system that supports multiple workflows. Primary cases:

• Just being able to flutter run
• Using local engine builds
• Working on the native application code (for which being able to use the underlying native build directly is likely to be easiest)

The iOS/macOS structure seems to strike a good balance of supporting all of that. I'd like the Windows and Linux builds to do balance them as well, so using that structure (where the flutter tool sets configuration and invokes the native build, and the native build has access to a script that copies necessary resources) seems like a good option.

In this case though the native build either needs to know the directory location to copy, or it needs to know the location of a script to invoke that can do so for it. This is where I'll plead ignorance a bit in that I don't see too much of a difference between the two (that said, i'll defer to your judgement here).

So I will create an asset sync script for Linux (and Windows) which can be invoked for either a bin/cache or local engine configuration. This will copy the sources into linux/flutter and windows/flutter. flutter build will be responsible for setting a generated_flutter_root file or a build mode override.

I think that should satisfy the requirement that the native build is still directly usable, while still delegating the heavy lifting to code controlled by the flutter tool

I don't see too much of a difference between the two

The main difference I see is that it gives us the ability to put a lot of logic in the tool itself, and change that logic over time without breaking things:

• Details of where the caches live and how they are updated
• Details of how we decide whether re-copying is necessary (for eventually fixing the alway-relink regression we've introduced to FDE, for instance)
• Understanding of how local build overrides work

So I will create an asset sync script for Linux (and Windows) which can be invoked for either a bin/cache or local engine configuration.

SGTM. We should be able to use environment variables to communicate local build overrides, as in the macOS script. For Windows, generated properties files can set values that will be in the build environment, and for Linux we can either do another file read in the Makefile like for the Flutter root (eventually I think we'll need a build system with an actual property management system that can replace using files like that), or just for now make the setting non-persistent on Linux and set in in the environment when invoking make from the tool.

TIL! Nice find :)

^^^

Having two entries for all seems confusing. But also this doesn't actually ever run the script (I'm guessing you didn't test this on a clean build?)

I think what we want is a:

.PHONY: flutterbuild
flutterbuild:
    $(FLUTTER_ROOT)/packages/flutter_tools/bin/linux_backend.sh

And rather than all depending on that, the steps that would fail without it should depend on it, otherwise parallel builds, or direct target builds, would fail. Changing the line below that currently depends on $(FLUTTER_COPY_STAMP_FILE) to depend on flutterbuild instead, since that's now the build step that creates those files, should do it.

That should be a order-only dependency ([...]: | flutterbuild) so that when we improve linux_backend.sh to not re-copy the files when it's not necessary, it won't still do unnecessary relinking.

Also, the $(FLUTTER_ASSETS_SOURCE) rule below that currently calls build build should be removed, and $(FLUTTER_ASSETS_SOURCE) should become part of the left hand side of the rule depending on flutterbuild

I tried to create a separate sync target and then remove the dependency on build bundle so we dont call it twice

You still have necessary files that don't have generation rules (everything that's still depending on the now-non-existent $(FLUTTER_COPY_STAMP_FILE) variable in your current patch; many scenarios would still fail. For instance, try, in a completely clean setup, running make build/linux/debug/flutter_desktop_example. (That's an unlikely scenario, but the same thing happens very easily if someone passes -j.)

I thought an order-only PHONY dependency wouldn't run multiple times; could you post the version with the dependencies where you saw the backend script firing repeatedly?

(Specifically, I thought this would work:

$(WRAPPER_SOURCES) $(FLUTTER_LIB): | sync

...

bundleflutterassets: | sync

Your other PR doesn't copy icudtl.dat to this location, so either it should do that, or this Makefile needs to know how to go get it from the Flutter cache internals. (My vote would be for the former.)

Fixed

Why export?

You still have necessary files that don't have generation rules (everything that's still depending on the now-non-existent $(FLUTTER_COPY_STAMP_FILE) variable in your current patch; many scenarios would still fail. For instance, try, in a completely clean setup, running make build/linux/debug/flutter_desktop_example. (That's an unlikely scenario, but the same thing happens very easily if someone passes -j.)

I thought an order-only PHONY dependency wouldn't run multiple times; could you post the version with the dependencies where you saw the backend script firing repeatedly?

(Specifically, I thought this would work:

$(WRAPPER_SOURCES) $(FLUTTER_LIB): | sync

...

bundleflutterassets: | sync

You should be able to remove |sync| here since it's a dependency of other things. It's not a top-level thing that we need independently, which is what |all| should depend on.

Add a comment here saying that it's phony because we don't know the inputs, so we trust the Flutter tooling to run only when necessary.