Created attachment 93642 [details]
Module that adds one menu and removes the Edit menu

Run this module.  Note the assertion error in
assert FileUtil.getConfigFile ("Menu/Edit") == null;
after the layer is merged.

The bug is here in SystemFileSystem lines 231 to 234, in core.startup - the code that creates the system filesystem:
FileSystem[] arr = new FileSystem[home == null ? 1 : 2];
arr[0] = new ModuleLayeredFileSystem(user, true, new FileSystem[0], false);
if (home != null) { //running with no userdir
   arr[1] = new ModuleLayeredFileSystem(home, false, extras, true);
}
return new SystemFileSystem (arr);

Notice the false which is passed if the userdir does not exist.  This calls:

    ModuleLayeredFileSystem (FileSystem writableLayer, boolean userDir, FileSystem[] otherLayers, boolean mgr) throws IOException {
        this(writableLayer, userDir, otherLayers, LayerCacheManager.manager(mgr));
    }

Notice the name of the boolean parameter is "userDir".  Now look at the name of that variable in the private constructor it invokes - it is now "addLookup"
    
    private ModuleLayeredFileSystem(FileSystem writableLayer, boolean addLookup, FileSystem[] otherLayers, LayerCacheManager mgr) throws IOException {
        this(writableLayer, addLookup, otherLayers, mgr, mgr.loadCache());
    }

and the real constructor sets the variable "addLayerBefore" to whether or not the userdir exists on disk (in a unit test environment it will not).

addLayerBefore controls whether dynamically added layers are added to the bottom of the system filesystem or the top
    
    private ModuleLayeredFileSystem(FileSystem writableLayer, boolean addLookup, FileSystem[] otherLayers, LayerCacheManager mgr, FileSystem cacheLayer) throws IOException {
        super(
            appendLayers(
                writableLayer, addLookup, otherLayers,
                cacheLayer == null ? mgr.createEmptyFileSystem() : cacheLayer,
                addLookup
            )
        );
...

appendLayers() inserts all FileSystems in the default lookup to the bottom of the layer stack if addLookup is true, and at the top if it is false.

This clearly looks like an accident.  The presence or absence of the userdir should have no effect on the way dynamically added layers are merged.  Either the constructor needs a separate argument (if it is ever actually useful to merge them underneath everything else - I suspect not), or the value should simply be defaulted to true.  It looks like somebody just used code completion and didn't notice they were passing the wrong variable value to appendLayers().

Since dynamically adding layer contents will almost always be to mask or unmask some files, I can't think of a use case for ever adding dynamic SFS contents below all other SFS contents.

Additionally, for some reason appendLayers() will only add a dynamic filesystem to the *top* if Boolean.TRUE.equals(fs.getRoot().getAttribute("fallback")) (this is very awkward, since you cannot set attributes of an FS root in an XML filesystem definition).  So it looks like the fix for bug 169892 caused some sort of regression here - there seem to be four problems:
1.  Presence or absence of userdir determines what position in the stacking order dynamic layers get, because presence or absence of the userdir is accidentally used to decide where such layers are placed.
2.  If addLookupBefore is true, filesystems with the "fallback" attribute true will be added, but filesystems with it set to false will never be.
3.  If addLookupBefore is false, filesystems with the "fallback" attribute false will be added, but layers with it set to true will never be
4.  The "fallback" attribute cannot be set in an XML file - it must be done programmatically.

All that analysis being done, I still cannot get masking to work, so there remains another problem :-(

Re "fallback":  Worse still, trying to set an attribute on the root of an XMLFileSystem *or* a MultiFileSystem will throw an IOException, because both are read-only.  So, to use this feature you have to create an extra MemoryFileSystem just to set an attribute on its root and merge it:

FileSystem mem = FileUtil.createMemoryFileSystem();
mem.getRoot().setAttribute("fallback", true);
setDelegates(new FileSystem[]{fs, mem});

(In reply to comment #2)
> dynamically adding layer contents will almost always be to mask or unmask
> some files

That has never been the use case to date. It is normally used to insert new files under some conditions (see ActiveConfigAction.DynLayer for an example). I doubt that masking from a dynamic layer was ever even tested as a possibility. That said, it would be nice for it to work. Marking P3 since this is not a primary use case.

> I can't think of a use case for ever adding dynamic SFS contents
> below all other SFS contents.

Ergonomics relies on exactly this if I recall correctly (it wants its placeholder layer to be overridden by newly enabled modules), hence the fallback facility. Yarda would of course know better.

> you cannot set attributes of an FS root in an XML filesystem definition

Sure you can (as of the 1.1 DTD). See ide.ergonomics/src/org/netbeans/modules/ide/ergonomics/fod/common.xml for an example:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE filesystem PUBLIC "-//NetBeans//DTD Filesystem 1.2//EN" "http://www.netbeans.org/dtds/filesystem-1_2.dtd">
<filesystem>
    <attr name="fallback" boolvalue="true"/>
</filesystem>

> it looks like the fix for bug 169892 caused some
> sort of regression here

Marking a blocker accordingly. BTW did you confirm this problem in 6.8 or only 6.9 dev?

> 2.  If addLookupBefore is true, filesystems with the "fallback" attribute true
> will be added, but filesystems with it set to false will never be.
> 3.  If addLookupBefore is false, filesystems with the "fallback" attribute
> false will be added, but layers with it set to true will never be

It's been a while since I worked on this code but I think the intent was to add the fallback layers only, then static layers, then the non-fallback layers only (or the reverse).

> The "fallback" attribute cannot be set in an XML file

Again, this is not true.

Without investigating too deeply I suggest Tim to see MFS#setPropagateMask and use it in his MFS. There is some duplicate, but I am not going to seek for it.

Tim, if you want to improve some documentation, feel free to do so.

Integrated into 'main-golden', will be available in build *201007290001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/c42bfa57c9d6
User: Jesse Glick <jglick@netbeans.org>
Log: Moving documentation about dynamic layer injection to Javadoc where it belongs.
Rearranging some arch.xml entries.
#179977: clarifying that setPropagateMasks is necessary to dynamically inject *_hidden files.