add missing export variable

[johnsonshih]

Add CLASS_LOADER_PUBLIC to ClassLoader::has_unmananged_instance_beencreated or it won't build on Windows

[mikaelarguedas]

@johnsonshih do you have an example where this is needed? or a reproducible example of this not building ?

I'm not fully familiar with visibility macros for Windows but it seems surprising to me that private attributes need to be declared public.

The code on the ros2 branch (that has the same set of visibility macros) seems to build fine on Windows 10 (VS2015 and VS2017).

[johnsonshih]

The build will fail when building shared_ptr_test.cpp with unresolved external symbol class_loader::ClassLoader::has_unmananged_instance_beencreated if we don't add the declaration. The name of CLASS_LOADER_PUBLIC macro is misleading, it's not to declare a entity public but specify the storage-class attribute when it's built on Windows (or visibility on non-Windows environment). The storage-class attribute (dllexport or dllimport)is used to tell the compiler if the current built component need to provide the entity instance. If the current built dll provide the entity, need to set to dllexport or nothing. If the current built dll consumes the entity, the attribute need to set to dllimport. has_unmananged_instance_beencreated is a static value, and is provided by Class_Loader.dll, so we need to use CLASS_LOADER_PUBLIC to specify the attribute so compiler know how to handle this value.

[kejxu]

hey @mikaelarguedas and maintainers, just a friendly ping, could this change be merged into the mainline?

[mikaelarguedas]

Thanks James for the reminder

Friendly ping @nuclearsandwich that is currently maintaining this package

On Fri, Jan 18, 2019, 13:37 James Xu <notifications@github.com wrote:

hey @mikaelarguedas https://github.com/mikaelarguedas and maintainers, just a friendly ping, could this change be merged into the mainline?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ros/class_loader/pull/102#issuecomment-455406891, or mute the thread https://github.com/notifications/unsubscribe-auth/AGVh8DFV569EdOXiWH0h4P1rFAqXznSVks5vETNLgaJpZM4VppUE .

[kejxu]

hi @nuclearsandwich, just a friendly ping so this pull request won't be lost in the mail box

[nuclearsandwich]

hi @nuclearsandwich, just a friendly ping so this pull request won't be lost in the mail box

Thanks @kejxu. As an update, the prerelease job is not behaving stably. Which is why these PRs are still in flight. I don't think any one of them is the cause, but I am still trying to get the pre-release workspace to build, let alone run tests.

[kejxu]

get the pre-release workspace to build, let alone run tests

My apologies for the spam, I thought the pre-release check was referring to the CI checks. Please take your time with the build =)

[nuclearsandwich]

My apologies for the spam, I thought the pre-release check was referring to the CI checks. Please take your time with the build =)

No worries. You had no way to know that work was still in progress as I hadn't written any update. CI can tell us that class_loader builds and passes all its own tests but until a release is actually made we won't know whether the changes break any downstream packages on the farm (at which point it is not quite too late). So the ros_buildfarm library supports a prerelease script which you can generate from the prerelease website which will build your packages from a target branch, then build and test the packages dependencies from source on top of that.

It functions very similarly to ci.ros2.org in that regard but the builds are run on your local machine.

[seanyen]

@nuclearsandwich ping! Just in case this got lost in your backlogs. 😃