Skip to content

Conversation

@wyattjoe40
Copy link

  • Currently there is an issue when an AAR file is updated, but
    the Android Binding library does not regenerate the bindings.
    I believe this is caused while copying over the AAR file contents
    to the library_project_imports directory. Currently, the logic is to
    only attempt a copy if the file doesn't exist. However, I think the
    correct logic should be to copy if the file has changed.
  • The result of this change is that my .JAR file, which resides inside
    of my .AAR file, will be correctly copied to the library_project_imports
    and the library_project_jars, and can be used to update the generated
    bindings.
  • This logic may also be incorrect in other places in the file.

- Currently there is an issue where an AAR file is updated, but
  the Android Binding library does not regenerate the bindings.
  I believe this is caused by the incorrect logic of checking
  if the file exists before trying to copy. We should not check
  if the file exists, but instead let the new file be copied over
  if it is indeed newer.

- This logic may be incorrect in other places in the file. However,
  I'm not confident enough to make that call.
@monojenkins
Copy link

Hello! I'm the build bot for the Mono project.

I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done.

Contributors can ignore this message.

@dnfclas
Copy link

dnfclas commented Jan 24, 2017

Hi @wyattjoe40, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, DNFBOT;

@wyattjoe40 wyattjoe40 closed this Jan 25, 2017
jonpryor pushed a commit that referenced this pull request May 31, 2017
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=56867

It turns out that having a *package* definition in the pkgdef
without a corresponding Package entry in the same file, makes
it impossible for VS to find the associated resources with the
given IDs (#400 for the icon, #110 and #112 for the name and
description).

It also turns out that even if were to do without the icon,
using plain strings for #110/112 doesn't work either, and the
package entry has to be there anyway, alongside the assembly
and code base.

Therefore, just add it to the VSIX payload so that VS can use
it to locate those resources for Help/About and nothing else.

Since we still tell the VSSDK targets *not* to create the PKGDEF
from the Package-derived class, this should still mean it's a
plain IL compilation for xbuild/MacOSX and hopefully it won't
break anything.

NOTE: the AssemblyName has to match what's in the PKGDEF, so
we made it match with the official one too. Since this assembly
name isn't used for anything at run-time, it's fine to leave it
as-is with no changes.

The new gazillion packages/references are needed just so the
AndroidSdkPackage class can inherit from Package as expected
by VS.
jonpryor pushed a commit to jonpryor/xamarin-android that referenced this pull request Jun 5, 2017
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=56867

It turns out that having a *package* definition in the pkgdef
without a corresponding Package entry in the same file, makes
it impossible for VS to find the associated resources with the
given IDs (dotnet#400 for the icon, dotnet#110 and dotnet#112 for the name and
description).

It also turns out that even if were to do without the icon,
using plain strings for dotnet#110/112 doesn't work either, and the
package entry has to be there anyway, alongside the assembly
and code base.

Therefore, just add it to the VSIX payload so that VS can use
it to locate those resources for Help/About and nothing else.

Since we still tell the VSSDK targets *not* to create the PKGDEF
from the Package-derived class, this should still mean it's a
plain IL compilation for xbuild/MacOSX and hopefully it won't
break anything.

NOTE: the AssemblyName has to match what's in the PKGDEF, so
we made it match with the official one too. Since this assembly
name isn't used for anything at run-time, it's fine to leave it
as-is with no changes.

The new gazillion packages/references are needed just so the
AndroidSdkPackage class can inherit from Package as expected
by VS.
jonpryor pushed a commit that referenced this pull request Jun 6, 2017
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=56867

It turns out that having a *package* definition in the pkgdef
without a corresponding Package entry in the same file, makes
it impossible for VS to find the associated resources with the
given IDs (#400 for the icon, #110 and #112 for the name and
description).

It also turns out that even if were to do without the icon,
using plain strings for #110/112 doesn't work either, and the
package entry has to be there anyway, alongside the assembly
and code base.

Therefore, just add it to the VSIX payload so that VS can use
it to locate those resources for Help/About and nothing else.

Since we still tell the VSSDK targets *not* to create the PKGDEF
from the Package-derived class, this should still mean it's a
plain IL compilation for xbuild/MacOSX and hopefully it won't
break anything.

NOTE: the AssemblyName has to match what's in the PKGDEF, so
we made it match with the official one too. Since this assembly
name isn't used for anything at run-time, it's fine to leave it
as-is with no changes.

The new gazillion packages/references are needed just so the
AndroidSdkPackage class can inherit from Package as expected
by VS.
@github-actions github-actions bot locked and limited conversation to collaborators Feb 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants