[Xamarin.Android.Build.Tasks] Path is too long

[dellis1972]

Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=30147

Windows (still) has a max path limit of 260 characters..
This can cause us a problem if the user creates a project in
a directory which already takes up most of that limit.. Like on
their Desktop!

This is because of the intermediate structure that was introduced
to handle embedding resources into the assemblies. The current
intermediate structure is as follows

`$(IntermediateOutputPath)\__library_project_imports__\$(AssemblyName)\library_project_imports\`
`$(IntermediateOutputPath)\__library_project_imports__\$(AssemblyName)\native_library_imports\`

Now consider that $(AssemblyName) can sometimes end up with something lile
"Xamarin.Android.Support.v7.AppCompat.21.0.3.0" you can easily see how we
start getting into trouble.

There was an attempt to fix this up a while ago (722dcc05) by introducing the
$(UseShortFileNames) property. However while this did introduce a
directory structure which was shorter is broke backwards compatability
with older versions of xamarin-android.

This is because of the structore of the zip files we that are embedded
into the assemblies. The current system just extracts the zip into
the $(IntermediateOutputPath)\__library_project_imports__\$(AssemblyName)\
directory and assumes that it contains a library_project_imports directory.

All the build tasks also assumed that as well.

So the fix needs to be done on a number of fronts. Firstly we need to
update the $(_LibraryProjectImportsDirectoryName) and
$(_NativeLibraryImportsDirectoryName) to be something shorter.
Next up we need to shorten "library_project_imports" to something
else verbose as well. This should cut down on the amount of the MAX_PATH
we chew up.

The real key to this is to NOT change the structure of the zip files in the
assemblies! So when we generate a zip from "jlibs" we make sure that the
folder in the zip is called "library_project_imports". And when we extract
the zip file we makle sure that "library_project_imports" is replaced by
the shorter name.
This will ensure that we are backward compatbile with older versions BUT
more importantly we get to use the shorter directory structure.
The files for native librarys are not extracted to disk but are extracted
from memory so as long as the structure remains the same i.e "native_library_imports"
that code does not need to change.

The other thing we need to do is to update ResolveLibraryProjectImports Task to
upgrade the system when it runs. So if we already have a "libraryprojectimports.cache"
in place we just use that as is. But if we re-run the ResolveLibraryProjectImports
task (due to a change or a clean build) we detect if we have the older structure in place
and just remove it.. Since we are going to regenerate the entire cache again anyway
we might as well start from scratch.

With this in place it becomes possible we can now enable $(UseShortFileNames)
by default!

So the new structure that is created is as follows

`$(IntermediateOutputPath)\lp\<id>\jl`
`$(IntermediateOutputPath)\lp\<id>\nl`

The will be a single integer value which can be mapped to the source
assembly via a new map.cache file. This .cache file will contain a list of
assemblys. We use the Index of the assembly in the list to figure out
which cache directory to use.

UnnamedProject
Library1

In this case Library1 would have an index of 1 and UnnamedProject will
be 0. The old behaviour can be enabled by setting
$(UseShortFileNames) to False.

Note in order to keep existing bindings generator behaviour consistent,
the BindingsGenerator task will not use the $(UseShortFileNames)
property to control how it generates its .cs files. Instead a new
property

`$(UseShortGeneratorFileNames)`

which can be used to control if the generator produces short names
(e.g 1.cs, 2.cs). This will be False by default.

[dellis1972]

Just wondering if we should come up with even shorter names for the projects to buy us as many extra characters as possible?

so "__library_project_imports" -> "lp" (rather than my proposed "lp")
"library_project_imports" -> "jl" (rather than the current "llibs")

[jonpryor]

If we're going to delete oldPath, don't we need to make the parent directory of oldPath writable as well, so that we can delete oldPath?

[jonpryor]

Comment-wise, it would be nice if the commit also described the new directory structure. :-)

What it looks like -- at a quick glance -- is that __library_project_imports__ becomes lp. This is a good start, but what about the nested native_library_imports and library_project_imports directories -- can those be shortened?

Possibly "even further!", should we "hash" the assembly name so that we have a consistent 8-character "assembly name" directory instead of the variable length setup it currently uses? (This might be significantly more complicated, but it's a thought!)

[jonpryor]

That moment when i find the the length of Xamarin.Android.Support.v7.AppCompat.21.0.3.0 is 46 characters, and this PR shortens things to 10.

ROFLMAO.

[jonpryor]

This presents a theoretical problem -- which presumably won't be all that theoretical? -- in which two assemblies have an md2 which is the same value. (This should plausibly be more common, since we're only taking the first 5 values in the md2 value! See MonoAndroidHelper.GetLibraryImportDirectoryNameForAssembly(). That said, I don't see any issues in my $HOME/.local/share/Xamarin, so maybe conflict isn't that likely?)

If we have two directories which hash to the same value, what should be done, and how can it be supported?

(Can it be supported?)

For example, assume that assemblies A1 and A2 have names which hash to the same value. The Directory.Delete() here will nuke the extraction of one, which will presumably result in a later build break, as e.g. some resource is used which doesn't exist.

Alternatively, instead of the Directory.Delete(), we could instead extract both files into the same directory. The problem with this approach is deleting entries which no longer exist (bleh). Alternatively, there would be a problem if both assemblies provide a resource of the same name, e.g. classes.jar (which iirc is a fairly common name).

Offhand, I'm not immediately sure if this is solvable. At the same time, I'm also not sure if its "fixable", or that it even should be fixed.

What might be a good idea, though, is to check for it: we have all of the Assemblies of interest, and therefore we can compute the abbreviated md2 for all of those assemblies, and check to see if there are any conflicts at all:

    var haveConflicts = new Dictionary<string, int>();
    foreach (var a in Assemblies) {
        var n = Xamarin.Android.Tasks.MonoAndroidHelper.GetLibraryImportDirectoryNameForAssembly (Path.GetFileNameWithoutExtension (a));
        int c;
        if (!haveConflicts.TryGetValue (n, out c)) {
            haveConflicts.Add (n, 1);
        }
        else haveConflicts [n] = c+1;
    }
    if (haveConflicts.Values.Any (v => v > 1))
        Log.LogError ("We have a hash conflict! Please rebuild with `$(UseShortFileNames)`=False.");

That's clearly not a great error message, but the idea is there.

[dellis1972]

The Hash has been replaced by a lookup cache file which will map the assembly to a integer index directory. So you get something like

MyProject=1
Xamarin.Android.Support.v7.AppCompat.21.0.3.0=2

So on the path we only take up "1" or "2" (up to the number of dependencies). Rather than a ton of redundant characters.

[jonpryor]

As elaborated on in an earlier comment/down below, I believe that the lookup cache file needs to instead have:

1=MyProject
2=Xamarin.Android.Support.v7.AppCompat.21.0.3.0

[jonpryor]

Smells like we should parameterize this test so that both native_library_imports ($(UseShortFileNames)=False)andnl ($(UseShortFileNames)`=True) are checked.

[jonpryor]

I think we could go all C# 6 on this and do:

var fullName = modifyCallback?.Invoke (entry.FullName) ?? entry.FullName;

:-D

[jonpryor]

Indentation here doesn't match surrounding text.

(The perils of a single file that uses both tabs and spaces...)

[jonpryor]

Not sure if this is important/matters, but e.g. in the previous <ResolveLibraryProjectImports/> invocation, we had:

<ResolveLibraryProjectImports
        OutputImportDirectory="$(IntermediateOutputPath)__library_projects__\"

That would now be:

<ResolveLibraryProjectImports
        OutputImportDirectory="$(IntermediateOutputPath)__library_projects__"

Note the removal of the trailing \.

Is this a problem? Possibly; I know we've encountered previous bugs where the presence/lack of a trailing / would alter behavior, causing the "wrong" directory to be deleted. For example, monodroid/a22d699b / Bug #27442, which references the "same" property!

For sanity, this property should also include a trailing \, both on this line and the following line.

[jonpryor]

This might be a variation on monodroid/a22d699b?

[jonpryor]

Should this actually be static? This gives me the heeby-jeebies. Are we sure this won't be "mis-used"? What happens when multiple projects are built within the IDE, could things get out-of-sync/inconsistent?

[jonpryor]

In retrospect, it "needs" to be static in order to allow GetLibraryImportDirectoryNameForAssembly() to work at all.

I'm still not sure that's a good idea, though. Perhaps we should introduce a new type, and make assemblyIdentityMap and GetLibraryImportDirectoryNameForAssembly() instance members on that new type? The problem with such an approach is that all tasks that require the map would need to create an instance of this type instead of sharing a static instance, but that approach would certainly alleviate my concerns about building multiple projects -- possibly concurrently! -- in the same process over time.

[dellis1972]

I'm gonna rework to use a wrapper class and no statics..
There will be a small overhead since we need to load that data from disk each time

[jonpryor]

I'm willing to accept that overhead because I have no idea what the ramifications of "too much static data" is in the larger ecosystem. That scares me.

[jonpryor]

You should throw InvalidOperationException, not Exception.

[jonpryor]

Question: What should happen if the assembly contains =? = isn't a restricted character; an assembly named foo=bar=baz.dll is perfectly kosher.

What will happen is that things will break two lines down, because the assumption is that there is only one = on a line. If there are more than one, the first item is the assembly, the second is the value, but we'd actually have a line containing:

foo=bar=baz=1

but then do assemblyIdentityMap.Add("foo", "bar"), which is wrong.

What we can instead do is reorder the values: instead of ASSEMBLY=DIRECTORY, do DIRECTOR=ASSEMBLY, then use the string.Split(char[], int) overload.

For example, if we instead had a line of:

1=foo=bar=baz.dll

Then line.Split(new[]{'='}, 2) will return new[]{ "1", "foo=bar=baz.dll" }, which is fine.

This implies that the file format needs "reversing".

[dellis1972]

if we really think about it we don't need a dictionary since we are just using the index of the item. So we just add the item names to a list in order and save that to disk. Then we can just using IndexOf to get the id.

I don't think there is any need to have a key=value.. apart from ease of reading.

[jonpryor]

Yup.

[jonpryor]

I half wonder if the diff would be smaller if, before this line, we did:

if (true) {

so that the indentation matched what we had before.

Even if this did result in a smaller diff, I'm not sure that would actually be a good idea. :-/

[jonpryor]

This type should be internal, if possible.

[dellis1972]

build