Skip to content

Manually add TypeForwardedTo in ref assemblies #14538

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Oct 8, 2019

Conversation

JunTaoLuo
Copy link
Contributor

We need to manually add TypeForwardedTo assembly attributes in ref assemblies for some packages. Tested locally:
image

WIP while confirming this resolves the original issue.

@JunTaoLuo JunTaoLuo added the Servicing-consider Shiproom approval is required for the issue label Sep 27, 2019
@JunTaoLuo JunTaoLuo added this to the 3.0.x milestone Sep 27, 2019
@@ -47,6 +47,7 @@
<PropertyGroup>
<_RefSourceOutputPath>$([System.IO.Directory]::GetParent('$(MSBuildProjectDirectory)'))/ref/</_RefSourceOutputPath>
<_RefSourceFileName>$(AssemblyName).$(TargetFramework).cs</_RefSourceFileName>
<_ManualRefSourceFileName>$(AssemblyName).Manual.cs</_ManualRefSourceFileName>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any typeforwards that are TFM specific? Like, in ns2.0 the type was still in assembly A, and then we moved it to assembly B for nca3.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not right now. I want to keep this simple for now, but I do recognize that there could potentially be TFM dependency manual content. If that arises, we'll need to make the Manual.cs files TFM specific. There are other clean up items I'd like to make so I'll file a followup issue.

Copy link
Member

@Pilchie Pilchie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you find the set of type forwards to add?

@JunTaoLuo JunTaoLuo marked this pull request as ready for review September 30, 2019 06:25
@JunTaoLuo
Copy link
Contributor Author

@Pilchie I searched for TypeForwardedTo assembly attributes in our *.cs files. Are there other ways of adding this attribute I'm not aware of (some MSBuild magic)?

@JunTaoLuo
Copy link
Contributor Author

This PR is ready for review and I'll follow up with the servicing request tomorrow.

A few items to potentially address as a followup:

  • How do we ensure everyone who added TypeForwardedTo attributes knows to add a Manual.cs file and set the property in the csproj?
  • I'll followup with corefx to see what mechanisms they have.

function r(e,t){if(e===t)return 0;for(var n=e.length,r=t.length,o=0,i=Math.min(n,r);o<i;++o)if(e[o]!==t[o]){n=e[o],r=t[o];break}return n<r?-1:r<n?1:0}function o(e){return t.Buffer&&"function"==typeof t.Buffer.isBuffer?t.Buffer.isBuffer(e):!(null==e||!e._isBuffer)}var i=n(35),s=Object.prototype.hasOwnProperty,a=Array.prototype.slice,c="foo"===function(){}.name;function u(e){return Object.prototype.toString.call(e)}function l(e){return!o(e)&&("function"==typeof t.ArrayBuffer&&("function"==typeof ArrayBuffer.isView?ArrayBuffer.isView(e):!!e&&(e instanceof DataView||!!(e.buffer&&e.buffer instanceof ArrayBuffer))))}var f=e.exports=v,h=/\s*function\s+([^\(\s]*)\s*/;function p(e){if(i.isFunction(e)){if(c)return e.name;var t=e.toString().match(h);return t&&t[1]}}function d(e,t){return"string"==typeof e?e.length<t?e:e.slice(0,t):e}function g(e){if(c||!i.isFunction(e))return i.inspect(e);var t=p(e);return"[Function"+(t?": "+t:"")+"]"}function y(e,t,n,r,o){throw new f.AssertionError({message:n,actual:e,expected:t,operator:r,stackStartFunction:o})}function v(e,t){e||y(e,!0,t,"==",f.ok)}function b(e,t,n,s){if(e===t)return!0;if(o(e)&&o(t))return 0===r(e,t);if(i.isDate(e)&&i.isDate(t))return e.getTime()===t.getTime();if(i.isRegExp(e)&&i.isRegExp(t))return e.source===t.source&&e.global===t.global&&e.multiline===t.multiline&&e.lastIndex===t.lastIndex&&e.ignoreCase===t.ignoreCase;if(null!==e&&"object"==typeof e||null!==t&&"object"==typeof t){if(l(e)&&l(t)&&u(e)===u(t)&&!(e instanceof Float32Array||e instanceof Float64Array))return 0===r(new Uint8Array(e.buffer),new Uint8Array(t.buffer));if(o(e)!==o(t))return!1;var c=(s=s||{actual:[],expected:[]}).actual.indexOf(e);return-1!==c&&c===s.expected.indexOf(t)||(s.actual.push(e),s.expected.push(t),function(e,t,n,r){if(null==e||null==t)return!1;if(i.isPrimitive(e)||i.isPrimitive(t))return e===t;if(n&&Object.getPrototypeOf(e)!==Object.getPrototypeOf(t))return!1;var o=m(e),s=m(t);if(o&&!s||!o&&s)return!1;if(o)return e=a.call(e),t=a.call(t),b(e,t,n);var c,u,l=S(e),f=S(t);if(l.length!==f.length)return!1;for(l.sort(),f.sort(),u=l.length-1;u>=0;u--)if(l[u]!==f[u])return!1;for(u=l.length-1;u>=0;u--)if(c=l[u],!b(e[c],t[c],n,r))return!1;return!0}(e,t,n,s))}return n?e===t:e==t}function m(e){return"[object Arguments]"==Object.prototype.toString.call(e)}function w(e,t){if(!e||!t)return!1;if("[object RegExp]"==Object.prototype.toString.call(t))return t.test(e);try{if(e instanceof t)return!0}catch(e){}return!Error.isPrototypeOf(t)&&!0===t.call({},e)}function E(e,t,n,r){var o;if("function"!=typeof t)throw new TypeError('"block" argument must be a function');"string"==typeof n&&(r=n,n=null),o=function(e){var t;try{e()}catch(e){t=e}return t}(t),r=(n&&n.name?" ("+n.name+").":".")+(r?" "+r:"."),e&&!o&&y(o,n,"Missing expected exception"+r);var s="string"==typeof r,a=!e&&o&&!n;if((!e&&i.isError(o)&&s&&w(o,n)||a)&&y(o,n,"Got unwanted exception"+r),e&&o&&n&&!w(o,n)||!e&&o)throw o}f.AssertionError=function(e){var t;this.name="AssertionError",this.actual=e.actual,this.expected=e.expected,this.operator=e.operator,e.message?(this.message=e.message,this.generatedMessage=!1):(this.message=d(g((t=this).actual),128)+" "+t.operator+" "+d(g(t.expected),128),this.generatedMessage=!0);var n=e.stackStartFunction||y;if(Error.captureStackTrace)Error.captureStackTrace(this,n);else{var r=new Error;if(r.stack){var o=r.stack,i=p(n),s=o.indexOf("\n"+i);if(s>=0){var a=o.indexOf("\n",s+1);o=o.substring(a+1)}this.stack=o}}},i.inherits(f.AssertionError,Error),f.fail=y,f.ok=v,f.equal=function(e,t,n){e!=t&&y(e,t,n,"==",f.equal)},f.notEqual=function(e,t,n){e==t&&y(e,t,n,"!=",f.notEqual)},f.deepEqual=function(e,t,n){b(e,t,!1)||y(e,t,n,"deepEqual",f.deepEqual)},f.deepStrictEqual=function(e,t,n){b(e,t,!0)||y(e,t,n,"deepStrictEqual",f.deepStrictEqual)},f.notDeepEqual=function(e,t,n){b(e,t,!1)&&y(e,t,n,"notDeepEqual",f.notDeepEqual)},f.notDeepStrictEqual=function e(t,n,r){b(t,n,!0)&&y(t,n,r,"notDeepStrictEqual",e)},f.strictEqual=function(e,t,n){e!==t&&y(e,t,n,"===",f.strictEqual)},f.notStrictEqual=function(e,t,n){e===t&&y(e,t,n,"!==",f.notStrictEqual)},f.throws=function(e,t,n){E(!0,e,t,n)},f.doesNotThrow=function(e,t,n){E(!1,e,t,n)},f.ifError=function(e){if(e)throw e};var S=Object.keys||function(e){var t=[];for(var n in e)s.call(e,n)&&t.push(n);return t}}).call(this,n(10))},function(e,t){e.exports=function(e){return e&&"object"==typeof e&&"function"==typeof e.copy&&"function"==typeof e.fill&&"function"==typeof e.readUInt8}},function(e,t){"function"==typeof Object.create?e.exports=function(e,t){e.super_=t,e.prototype=Object.create(t.prototype,{constructor:{value:e,enumerable:!1,writable:!0,configurable:!0}})}:e.exports=function(e,t){e.super_=t;var n=function(){};n.prototype=t.prototype,e.prototype=new n,e.prototype.constructor=e}},function(e,t,n){e.exports=n(11)},function(e,t){var n={}.toString;e.exports=Array.isArray||function(e){return"[object Array]"==n.call(e)}},function(e,t){},function(e,t,n){"use strict";var r=n(14).Buffer,o=n(60);e.exports=function(){function e(){!function(e,t){if(!(e instanceof t))throw new TypeError("Cannot call a class as a function")}(this,e),this.head=null,this.tail=null,this.length=0}return e.prototype.push=function(e){var t={data:e,next:null};this.length>0?this.tail.next=t:this.head=t,this.tail=t,++this.length},e.prototype.unshift=function(e){var t={data:e,next:this.head};0===this.length&&(this.tail=t),this.head=t,++this.length},e.prototype.shift=function(){if(0!==this.length){var e=this.head.data;return 1===this.length?this.head=this.tail=null:this.head=this.head.next,--this.length,e}},e.prototype.clear=function(){this.head=this.tail=null,this.length=0},e.prototype.join=function(e){if(0===this.length)return"";for(var t=this.head,n=""+t.data;t=t.next;)n+=e+t.data;return n},e.prototype.concat=function(e){if(0===this.length)return r.alloc(0);if(1===this.length)return this.head.data;for(var t,n,o,i=r.allocUnsafe(e>>>0),s=this.head,a=0;s;)t=s.data,n=i,o=a,t.copy(n,o),a+=s.data.length,s=s.next;return i},e}(),o&&o.inspect&&o.inspect.custom&&(e.exports.prototype[o.inspect.custom]=function(){var e=o.inspect({length:this.length});return this.constructor.name+" "+e})},function(e,t){},function(e,t,n){var r=n(6),o=r.Buffer;function i(e,t){for(var n in e)t[n]=e[n]}function s(e,t,n){return o(e,t,n)}o.from&&o.alloc&&o.allocUnsafe&&o.allocUnsafeSlow?e.exports=r:(i(r,t),t.Buffer=s),i(o,s),s.from=function(e,t,n){if("number"==typeof e)throw new TypeError("Argument must not be a number");return o(e,t,n)},s.alloc=function(e,t,n){if("number"!=typeof e)throw new TypeError("Argument must be a number");var r=o(e);return void 0!==t?"string"==typeof n?r.fill(t,n):r.fill(t):r.fill(0),r},s.allocUnsafe=function(e){if("number"!=typeof e)throw new TypeError("Argument must be a number");return o(e)},s.allocUnsafeSlow=function(e){if("number"!=typeof e)throw new TypeError("Argument must be a number");return r.SlowBuffer(e)}},function(e,t,n){(function(e){var r=void 0!==e&&e||"undefined"!=typeof self&&self||window,o=Function.prototype.apply;function i(e,t){this._id=e,this._clearFn=t}t.setTimeout=function(){return new i(o.call(setTimeout,r,arguments),clearTimeout)},t.setInterval=function(){return new i(o.call(setInterval,r,arguments),clearInterval)},t.clearTimeout=t.clearInterval=function(e){e&&e.close()},i.prototype.unref=i.prototype.ref=function(){},i.prototype.close=function(){this._clearFn.call(r,this._id)},t.enroll=function(e,t){clearTimeout(e._idleTimeoutId),e._idleTimeout=t},t.unenroll=function(e){clearTimeout(e._idleTimeoutId),e._idleTimeout=-1},t._unrefActive=t.active=function(e){clearTimeout(e._idleTimeoutId);var t=e._idleTimeout;t>=0&&(e._idleTimeoutId=setTimeout(function(){e._onTimeout&&e._onTimeout()},t))},n(63),t.setImmediate="undefined"!=typeof self&&self.setImmediate||void 0!==e&&e.setImmediate||this&&this.setImmediate,t.clearImmediate="undefined"!=typeof self&&self.clearImmediate||void 0!==e&&e.clearImmediate||this&&this.clearImmediate}).call(this,n(10))},function(e,t,n){(function(e,t){!function(e,n){"use strict";if(!e.setImmediate){var r,o,i,s,a,c=1,u={},l=!1,f=e.document,h=Object.getPrototypeOf&&Object.getPrototypeOf(e);h=h&&h.setTimeout?h:e,"[object process]"==={}.toString.call(e.process)?r=function(e){t.nextTick(function(){d(e)})}:!function(){if(e.postMessage&&!e.importScripts){var t=!0,n=e.onmessage;return e.onmessage=function(){t=!1},e.postMessage("","*"),e.onmessage=n,t}}()?e.MessageChannel?((i=new MessageChannel).port1.onmessage=function(e){d(e.data)},r=function(e){i.port2.postMessage(e)}):f&&"onreadystatechange"in f.createElement("script")?(o=f.documentElement,r=function(e){var t=f.createElement("script");t.onreadystatechange=function(){d(e),t.onreadystatechange=null,o.removeChild(t),t=null},o.appendChild(t)}):r=function(e){setTimeout(d,0,e)}:(s="setImmediate$"+Math.random()+"$",a=function(t){t.source===e&&"string"==typeof t.data&&0===t.data.indexOf(s)&&d(+t.data.slice(s.length))},e.addEventListener?e.addEventListener("message",a,!1):e.attachEvent("onmessage",a),r=function(t){e.postMessage(s+t,"*")}),h.setImmediate=function(e){"function"!=typeof e&&(e=new Function(""+e));for(var t=new Array(arguments.length-1),n=0;n<t.length;n++)t[n]=arguments[n+1];var o={callback:e,args:t};return u[c]=o,r(c),c++},h.clearImmediate=p}function p(e){delete u[e]}function d(e){if(l)setTimeout(d,0,e);else{var t=u[e];if(t){l=!0;try{!function(e){var t=e.callback,r=e.args;switch(r.length){case 0:t();break;case 1:t(r[0]);break;case 2:t(r[0],r[1]);break;case 3:t(r[0],r[1],r[2]);break;default:t.apply(n,r)}}(t)}finally{p(e),l=!1}}}}}("undefined"==typeof self?void 0===e?this:e:self)}).call(this,n(10),n(19))},function(e,t,n){(function(t){function n(e){try{if(!t.localStorage)return!1}catch(e){return!1}var n=t.localStorage[e];return null!=n&&"true"===String(n).toLowerCase()}e.exports=function(e,t){if(n("noDeprecation"))return e;var r=!1;return function(){if(!r){if(n("throwDeprecation"))throw new Error(t);n("traceDeprecation")?console.trace(t):console.warn(t),r=!0}return e.apply(this,arguments)}}}).call(this,n(10))},function(e,t,n){"use strict";var r=n(66).Transform,o=n(15),i=n(22);function s(e){(e=e||{}).objectMode=!0,e.highWaterMark=16,r.call(this,e),this._msgpack=e.msgpack}function a(e){if(!(this instanceof a))return(e=e||{}).msgpack=this,new a(e);s.call(this,e)}function c(e){if(!(this instanceof c))return(e=e||{}).msgpack=this,new c(e);s.call(this,e),this._chunks=i()}o(s,r),o(a,s),a.prototype._transform=function(e,t,n){var r=null;try{r=this._msgpack.encode(e).slice(0)}catch(e){return this.emit("error",e),n()}this.push(r),n()},o(c,s),c.prototype._transform=function(e,t,n){e&&this._chunks.append(e);try{var r=this._msgpack.decode(this._chunks);this.push(r)}catch(e){return void(e instanceof this._msgpack.IncompleteBufferError?n():this.emit("error",e))}this._chunks.length>0?this._transform(null,t,n):n()},e.exports.decoder=c,e.exports.encoder=a},function(e,t,n){(t=e.exports=n(36)).Stream=t,t.Readable=t,t.Writable=n(41),t.Duplex=n(11),t.Transform=n(42),t.PassThrough=n(67)},function(e,t,n){"use strict";e.exports=i;var r=n(42),o=n(20);function i(e){if(!(this instanceof i))return new i(e);r.call(this,e)}o.inherits=n(15),o.inherits(i,r),i.prototype._transform=function(e,t,n){n(null,e)}},function(e,t,n){var r=n(22);function o(e){Error.call(this),Error.captureStackTrace&&Error.captureStackTrace(this,this.constructor),this.name=this.constructor.name,this.message=e||"unable to decode"}n(35).inherits(o,Error),e.exports=function(e){return function(e){e instanceof r||(e=r().append(e));var t=i(e);if(t)return e.consume(t.bytesConsumed),t.value;throw new o};function t(e,t,n){return t>=n+e}function n(e,t){return{value:e,bytesConsumed:t}}function i(e,r){r=void 0===r?0:r;var o=e.length-r;if(o<=0)return null;var i,l,f,h=e.readUInt8(r),p=0;if(!function(e,t){var n=function(e){switch(e){case 196:return 2;case 197:return 3;case 198:return 5;case 199:return 3;case 200:return 4;case 201:return 6;case 202:return 5;case 203:return 9;case 204:return 2;case 205:return 3;case 206:return 5;case 207:return 9;case 208:return 2;case 209:return 3;case 210:return 5;case 211:return 9;case 212:return 3;case 213:return 4;case 214:return 6;case 215:return 10;case 216:return 18;case 217:return 2;case 218:return 3;case 219:return 5;case 222:return 3;default:return-1}}(e);return!(-1!==n&&t<n)}(h,o))return null;switch(h){case 192:return n(null,1);case 194:return n(!1,1);case 195:return n(!0,1);case 204:return n(p=e.readUInt8(r+1),2);case 205:return n(p=e.readUInt16BE(r+1),3);case 206:return n(p=e.readUInt32BE(r+1),5);case 207:for(f=7;f>=0;f--)p+=e.readUInt8(r+f+1)*Math.pow(2,8*(7-f));return n(p,9);case 208:return n(p=e.readInt8(r+1),2);case 209:return n(p=e.readInt16BE(r+1),3);case 210:return n(p=e.readInt32BE(r+1),5);case 211:return n(p=function(e,t){var n=128==(128&e[t]);if(n)for(var r=1,o=t+7;o>=t;o--){var i=(255^e[o])+r;e[o]=255&i,r=i>>8}var s=e.readUInt32BE(t+0),a=e.readUInt32BE(t+4);return(4294967296*s+a)*(n?-1:1)}(e.slice(r+1,r+9),0),9);case 202:return n(p=e.readFloatBE(r+1),5);case 203:return n(p=e.readDoubleBE(r+1),9);case 217:return t(i=e.readUInt8(r+1),o,2)?n(p=e.toString("utf8",r+2,r+2+i),2+i):null;case 218:return t(i=e.readUInt16BE(r+1),o,3)?n(p=e.toString("utf8",r+3,r+3+i),3+i):null;case 219:return t(i=e.readUInt32BE(r+1),o,5)?n(p=e.toString("utf8",r+5,r+5+i),5+i):null;case 196:return t(i=e.readUInt8(r+1),o,2)?n(p=e.slice(r+2,r+2+i),2+i):null;case 197:return t(i=e.readUInt16BE(r+1),o,3)?n(p=e.slice(r+3,r+3+i),3+i):null;case 198:return t(i=e.readUInt32BE(r+1),o,5)?n(p=e.slice(r+5,r+5+i),5+i):null;case 220:return o<3?null:(i=e.readUInt16BE(r+1),s(e,r,i,3));case 221:return o<5?null:(i=e.readUInt32BE(r+1),s(e,r,i,5));case 222:return i=e.readUInt16BE(r+1),a(e,r,i,3);case 223:throw new Error("map too big to decode in JS");case 212:return c(e,r,1);case 213:return c(e,r,2);case 214:return c(e,r,4);case 215:return c(e,r,8);case 216:return c(e,r,16);case 199:return i=e.readUInt8(r+1),l=e.readUInt8(r+2),t(i,o,3)?u(e,r,l,i,3):null;case 200:return i=e.readUInt16BE(r+1),l=e.readUInt8(r+3),t(i,o,4)?u(e,r,l,i,4):null;case 201:return i=e.readUInt32BE(r+1),l=e.readUInt8(r+5),t(i,o,6)?u(e,r,l,i,6):null}if(144==(240&h))return s(e,r,i=15&h,1);if(128==(240&h))return a(e,r,i=15&h,1);if(160==(224&h))return t(i=31&h,o,1)?n(p=e.toString("utf8",r+1,r+i+1),i+1):null;if(h>=224)return n(p=h-256,1);if(h<128)return n(h,1);throw new Error("not implemented yet")}function s(e,t,r,o){var s,a=[],c=0;for(t+=o,s=0;s<r;s++){var u=i(e,t);if(!u)return null;a.push(u.value),t+=u.bytesConsumed,c+=u.bytesConsumed}return n(a,o+c)}function a(e,t,r,o){var s,a={},c=0;for(t+=o,s=0;s<r;s++){var u=i(e,t);if(!u)return null;var l=i(e,t+=u.bytesConsumed);if(!l)return null;a[u.value]=l.value,t+=l.bytesConsumed,c+=u.bytesConsumed+l.bytesConsumed}return n(a,o+c)}function c(e,t,n){var r=e.readInt8(t+1);return u(e,t,r,n,2)}function u(t,r,o,i,s){var a,c;if(r+=s,o<0)switch(o){case-1:return function(e,t,r){var o,i;switch(i=0,t){case 4:o=e.readUInt32BE(0);break;case 8:var s=e.readUInt32BE(0),a=e.readUInt32BE(4);i=s/4,o=(3&s)*Math.pow(2,32)+a;break;case 12:throw new Error("timestamp 96 is not yet implemented")}var c=1e3*o+Math.round(i/1e6);return n(new Date(c),t+r)}(c=t.slice(r,r+i),i,s)}for(a=0;a<e.length;a++){if(o===e[a].type)return c=t.slice(r,r+i),n(e[a].decode(c),s+i)}throw new Error("unable to find ext type "+o)}},e.exports.IncompleteBufferError=o},function(e,t,n){"use strict";var r=n(14).Buffer,o=n(22),i=.1;function s(e,t){var n;return(n=r.allocUnsafe(5))[0]=202,n.writeFloatBE(e,1),(t||Math.abs(e-n.readFloatBE(1))>i)&&((n=r.allocUnsafe(9))[0]=203,n.writeDoubleBE(e,1)),n}e.exports=function(e,t,n,i){function a(c,u){var l,f,h;if(void 0===c)throw new Error("undefined is not encodable in msgpack!");if(null===c)(l=r.allocUnsafe(1))[0]=192;else if(!0===c)(l=r.allocUnsafe(1))[0]=195;else if(!1===c)(l=r.allocUnsafe(1))[0]=194;else if("string"==typeof c)(f=r.byteLength(c))<32?((l=r.allocUnsafe(1+f))[0]=160|f,f>0&&l.write(c,1)):f<=255&&!n?((l=r.allocUnsafe(2+f))[0]=217,l[1]=f,l.write(c,2)):f<=65535?((l=r.allocUnsafe(3+f))[0]=218,l.writeUInt16BE(f,1),l.write(c,3)):((l=r.allocUnsafe(5+f))[0]=219,l.writeUInt32BE(f,1),l.write(c,5));else if(c&&(c.readUInt32LE||c instanceof Uint8Array))c instanceof Uint8Array&&(c=r.from(c)),c.length<=255?((l=r.allocUnsafe(2))[0]=196,l[1]=c.length):c.length<=65535?((l=r.allocUnsafe(3))[0]=197,l.writeUInt16BE(c.length,1)):((l=r.allocUnsafe(5))[0]=198,l.writeUInt32BE(c.length,1)),l=o([l,c]);else if(Array.isArray(c))c.length<16?(l=r.allocUnsafe(1))[0]=144|c.length:c.length<65536?((l=r.allocUnsafe(3))[0]=220,l.writeUInt16BE(c.length,1)):((l=r.allocUnsafe(5))[0]=221,l.writeUInt32BE(c.length,1)),l=c.reduce(function(e,t){return e.append(a(t,!0)),e},o().append(l));else{if(!i&&"function"==typeof c.getDate)return function(e){var t,n=1*e,i=Math.floor(n/1e3),s=1e6*(n-1e3*i);if(s||i>4294967295){(t=new r(10))[0]=215,t[1]=-1;var a=4*s,c=i/Math.pow(2,32),u=a+c&4294967295,l=4294967295&i;t.writeInt32BE(u,2),t.writeInt32BE(l,6)}else(t=new r(6))[0]=214,t[1]=-1,t.writeUInt32BE(Math.floor(n/1e3),2);return o().append(t)}(c);if("object"==typeof c)l=function(t){var n,i,s=-1,a=[];for(n=0;n<e.length;n++)if(e[n].check(t)){i=e[n].encode(t);break}if(!i)return null;1==(s=i.length-1)?a.push(212):2===s?a.push(213):4===s?a.push(214):8===s?a.push(215):16===s?a.push(216):s<256?(a.push(199),a.push(s)):s<65536?(a.push(200),a.push(s>>8),a.push(255&s)):(a.push(201),a.push(s>>24),a.push(s>>16&255),a.push(s>>8&255),a.push(255&s));return o().append(r.from(a)).append(i)}(c)||function(e){var t,n,i=[],s=0;for(t in e)e.hasOwnProperty(t)&&void 0!==e[t]&&"function"!=typeof e[t]&&(++s,i.push(a(t,!0)),i.push(a(e[t],!0)));s<16?(n=r.allocUnsafe(1))[0]=128|s:((n=r.allocUnsafe(3))[0]=222,n.writeUInt16BE(s,1));return i.unshift(n),i.reduce(function(e,t){return e.append(t)},o())}(c);else if("number"==typeof c){if((h=c)!==Math.floor(h))return s(c,t);if(c>=0)if(c<128)(l=r.allocUnsafe(1))[0]=c;else if(c<256)(l=r.allocUnsafe(2))[0]=204,l[1]=c;else if(c<65536)(l=r.allocUnsafe(3))[0]=205,l.writeUInt16BE(c,1);else if(c<=4294967295)(l=r.allocUnsafe(5))[0]=206,l.writeUInt32BE(c,1);else{if(!(c<=9007199254740991))return s(c,!0);(l=r.allocUnsafe(9))[0]=207,function(e,t){for(var n=7;n>=0;n--)e[n+1]=255&t,t/=256}(l,c)}else if(c>=-32)(l=r.allocUnsafe(1))[0]=256+c;else if(c>=-128)(l=r.allocUnsafe(2))[0]=208,l.writeInt8(c,1);else if(c>=-32768)(l=r.allocUnsafe(3))[0]=209,l.writeInt16BE(c,1);else if(c>-214748365)(l=r.allocUnsafe(5))[0]=210,l.writeInt32BE(c,1);else{if(!(c>=-9007199254740991))return s(c,!0);(l=r.allocUnsafe(9))[0]=211,function(e,t,n){var r=n<0;r&&(n=Math.abs(n));var o=n%4294967296,i=n/4294967296;if(e.writeUInt32BE(Math.floor(i),t+0),e.writeUInt32BE(o,t+4),r)for(var s=1,a=t+7;a>=t;a--){var c=(255^e[a])+s;e[a]=255&c,s=c>>8}}(l,1,c)}}}if(!l)throw new Error("not implemented yet");return u?l:l.slice()}return a}},function(e,t,n){"use strict";var r=this&&this.__awaiter||function(e,t,n,r){return new(n||(n=Promise))(function(o,i){function s(e){try{c(r.next(e))}catch(e){i(e)}}function a(e){try{c(r.throw(e))}catch(e){i(e)}}function c(e){e.done?o(e.value):new n(function(t){t(e.value)}).then(s,a)}c((r=r.apply(e,t||[])).next())})},o=this&&this._
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty annoying but expected. cc@rynowak. Eventually we want to ensure a version update doesn't require a code change in this file #11592

@Tratcher
Copy link
Member

So this entire setup is manual, adding a type forward requires editing two or three files? As a temporary fix to 3.0 I understand how we ended up here, but for every future version this is going to be a maintenance nightmare.

The bare minimum requirement for this process in 3.1 needs to be adding an enforcement mechanism (e.g. code check makes sure everything is in sync). Even with that, this is going to prove painful unless it can be auto-generated like the rest of the ref assemblies.

@Tratcher
Copy link
Member

Issue links?

@JunTaoLuo
Copy link
Contributor Author

Yes I filed dotnet/arcade#4026 for a feature request. I'll file followup issues for enhancement improvements when this PR is complete.

@safern
Copy link
Member

safern commented Sep 30, 2019

@JunTaoLuo you guys should also consider enabling ApiCompat in your builds to catch this kind of issues early: https://github.com/dotnet/arcade/tree/master/src/Microsoft.DotNet.ApiCompat

@Pilchie Pilchie added the feature-platform Deprecated: Cross-cutting issues related to ASP.NET Core as a platform label Sep 30, 2019
@JunTaoLuo JunTaoLuo force-pushed the johluo/ref-typeforward branch from 193601c to 204e8ac Compare October 1, 2019 16:26
@leecow leecow modified the milestones: 3.0.x, 3.0.1 Oct 3, 2019
@JunTaoLuo
Copy link
Contributor Author

A few unresolved issues:

  1. We've decided that MSB3243 warnings are benign given the way we build our ref packs. However, I can't seem to disable the these build warnings specifically using NoWarn. Looking at the source code I tried setting ResolveAssemblyReferencesSilent property to true. This works but in the future it may hide other issues.
  2. I see file version differences in FrameworkList.xml:
    image
    Assembly file versions seem to be generated based on the package version so it doesn't surprise me that this will be updated. Can someone confirm?
  3. I see package version differences in PackageOverrides.txt:
    image
    These seem like package versions so I'm thinking that this file looks correct? cc @nguerrera @dsplaisted since I'm not sure how this file is used in conflict resolution.
  4. I see assembly version and file version differences in PlatformManifest.txt:
    image
    The current build logic takes the PlatformManifest.txt generated by the .Runtime project which builds the shared framework with the implementation assemblies. If we want to use assembly versions of the ref assemblies instead, which would make sense to me, I'll need to update the logic. cc @nguerrera @dsplaisted since I'm not sure how this file is used in conflict resolution. Also, the same assembly file version issue as 2.

@dougbu
Copy link
Contributor

dougbu commented Oct 4, 2019

Assembly file versions seem to be generated based on the package version so it doesn't surprise me that this will be updated. Can someone confirm?

I believe assembly file versions are mostly ignored and agree they're generated based on package versions. As long as the source package version is of the .Ref package and not .Runtime package, I think we're good. But, to be sure, I defer to @nguerrera and @dsplaisted on this point too.

If we want to use assembly versions of the ref assemblies instead, which would make sense to me

We absolutely want to use assembly versions of the ref/ assemblies in the .Ref package as well as the wrapping installers. To do anything else would just muddle things.

@nguerrera
Copy link
Contributor

nguerrera commented Oct 4, 2019

You should be able to use MSB3243. (I've argued before that we should support NoWarn, but we don't. See dotnet/msbuild#4421).

That said, can you share a .binlog with these warnings being generated, I'd like to confirm they are truly benign and see if they can be prevented instead of suppressed.

@nguerrera
Copy link
Contributor

nguerrera commented Oct 4, 2019

I believe the package versions in PlatformManifest should remain at 3.0.0. Otherwise, if I ref 3.0 in a framework-dependent app + a 3.0.1 package, it won't deploy the 3.0.1 package and it would use the 3.0.0 in the shared framework if run on an unpatched machine.

@nguerrera
Copy link
Contributor

nguerrera commented Oct 4, 2019

I think it is fine for the file versions to change, as long as they match the reference assembly file versions.

EDIT: The file versions should increase here. If ever there is a situation where these reference assemblies can be seen in a package or elsewhere than the ref pack, and the greater file version would win conflict resolution as a final tie breaker, which is what we'd want because the greater file version has the bug fix.

@nguerrera
Copy link
Contributor

nguerrera commented Oct 4, 2019

+1 that AssemblyVersion in FrameworkList must match the AssemblyVersion of reference assembly and remain at 3.0.0.0

@nguerrera
Copy link
Contributor

cc @ericstj

@JunTaoLuo JunTaoLuo force-pushed the johluo/ref-typeforward branch from 5685aa4 to cf902de Compare October 5, 2019 09:16
@JunTaoLuo
Copy link
Contributor Author

The release/3.0 is broken: https://dev.azure.com/dnceng/public/_build/results?buildId=377534&view=logs&jobId=1b89928a-2219-5ef9-602f-f95beb3da4dc. Even though the Windows x64/x86 build shows as successful, there is the same failure message as this PR:

The element 'metadata' in namespace 'http://schemas.microsoft.com/packaging/2011/08/nuspec.xsd' has invalid child element 'icon' in namespace 'http://schemas.microsoft.com/packaging/2011/08/nuspec.xsd'. List of possible elements expected: 'description, frameworkAssemblies, dependencies, language, copyright, contentFiles, releaseNotes, summary, serviceable, requireLicenseAcceptance, iconUrl, packageTypes, developmentDependency, frameworkReferences, license, references, repository, tags' in namespace 'http://schemas.microsoft.com/packaging/2011/08/nuspec.xsd'. This validation error occurred in a 'icon' element.
F:\workspace\_work\1\s\src\Installers\Windows\SharedFramework\SharedFramework.wixproj(99,5): error MSB3073: The command "powershell -NoProfile -NoLogo F:\workspace\_work\1\s\\src\Installers\Windows\GenerateNugetPackageWithMsi.ps1 ^
F:\workspace\_work\1\s\src\Installers\Windows\SharedFramework\SharedFramework.wixproj(99,5): error MSB3073:                       'AspNetCore.SharedFramework' ^
F:\workspace\_work\1\s\src\Installers\Windows\SharedFramework\SharedFramework.wixproj(99,5): error MSB3073:                       'F:\workspace\_work\1\s\artifacts\installers\Release\aspnetcore-runtime-3.0.1-ci-win-x86.msi' ^
F:\workspace\_work\1\s\src\Installers\Windows\SharedFramework\SharedFramework.wixproj(99,5): error MSB3073:                       'F:\workspace\_work\1\s\artifacts\installers\Release\sfx_x86.cab' ^
F:\workspace\_work\1\s\src\Installers\Windows\SharedFramework\SharedFramework.wixproj(99,5): error MSB3073:                       'F:\workspace\_work\1\s\\src\Installers\Windows\SharedFramework\SharedFrameworkPackage.nuspec' ^
F:\workspace\_work\1\s\src\Installers\Windows\SharedFramework\SharedFramework.wixproj(99,5): error MSB3073:                       'F:\workspace\_work\1\s\artifacts\packages\Release\NonShipping\' ^
F:\workspace\_work\1\s\src\Installers\Windows\SharedFramework\SharedFramework.wixproj(99,5): error MSB3073:                       'x86' ^
F:\workspace\_work\1\s\src\Installers\Windows\SharedFramework\SharedFramework.wixproj(99,5): error MSB3073:                       '3.0.1-ci' ^
F:\workspace\_work\1\s\src\Installers\Windows\SharedFramework\SharedFramework.wixproj(99,5): error MSB3073:                       'F:\workspace\_work\1\s\' ^
F:\workspace\_work\1\s\src\Installers\Windows\SharedFramework\SharedFramework.wixproj(99,5): error MSB3073:                       '3' ^
F:\workspace\_work\1\s\src\Installers\Windows\SharedFramework\SharedFramework.wixproj(99,5): error MSB3073:                       '0'" exited with code 1.

Looks like the NuGet client brought in is not new enough

@JunTaoLuo
Copy link
Contributor Author

@wtgodbe I see that the current version of nuget.exe that we are using is 5.2.0 which doesn't have icon support yet: NuGet/Home#8660. I'll revert the changes in wixprojs since they download a nuget.exe directly here: https://github.com/aspnet/AspNetCore/blob/release/3.0/src/Installers/Windows/GenerateNugetPackageWithMsi.ps1#L27. I don't like how we are using a nuget.exe here but I do realize that nuget pack with nuspec isn't supported by dotnet nuget or dotnet pack (at least a while ago, but I'm not sure if support has been added).

@JunTaoLuo
Copy link
Contributor Author

@nguerrera looks like we build the targeting pack installers as well. I'm going to double check that they have the correct contents ASAP.

@dougbu
Copy link
Contributor

dougbu commented Oct 6, 2019

@rrelyea why does https://dist.nuget.org/win-x86-commandline/latest/nuget.exe point to the 5.2.0 client and not the 5.3.0 one? From https://docs.microsoft.com/en-us/nuget/release-notes/nuget-5.3 it seems 5.3.0 supports <icon>.

Separately, could we now be using dotnet pack in our .wixproj files i.e. is the following old news?

nuget pack with nuspec isn't supported by dotnet nuget or dotnet pack

@rrelyea
Copy link

rrelyea commented Oct 6, 2019

We usually wait a few weeks before we bless new versions of nuget.exe .
When we bless them, we turn on three auto update mechanisms:

  1. update tools.json per these docs: https://docs.microsoft.com/en-us/nuget/api/tools-json
  2. update the nuget.exe version behind the "Latest" url: https://dist.nuget.org/win-x86-commandline/latest/nuget.exe
  3. update nuget.commandline package on nuget.org, which feeds "nuget update -self"

This allows us to test new versions in the wild for a couple weeks before we auto-update people using these mechanisms to download nuget.exe

@dougbu
Copy link
Contributor

dougbu commented Oct 6, 2019

Thanx @rrelyea. What does that mean as far as timing of the 2nd update?

@rrelyea
Copy link

rrelyea commented Oct 6, 2019

We will likely bless 5.3.x this coming week, as long as feedback is all still positive

@nguerrera
Copy link
Contributor

Regarding this: #14538 (comment)

You should not ship anything in a package compilation assets that reference things in the shared framework at version > 3.0.0.0. If you have a package with a frameworkreference, the versions of the assembly refs in the compilation assets of that package must match the assembly versions of the framework ref pack.

So I don't think 3.0.1 packages should be compiled against 3.0.1 implementation assemblies, but rather against 3.0.0 reference assemblies. It should be the equivalent of what you'd get if you were to build this package in a separate repo and use a FrameworkReference.

I seem to recall in the 2.x days, there was effort done to make package servicing "jagged": only shipping serviced packages when there were changes, and not having depedency versions all line up to the patch version. I think the same sort of thing is still required for 3.0.x.

Another possibility, which is hacky, would be to pin the implementation assembly version to 3.0.0.0 as well...

I am really not feeling well, so tagging @dsplaisted and @ericstj to help with the follow up here.

@dsplaisted
Copy link
Member

@JunTaoLuo Yes, I think anything that ships in a package should build against the reference assemblies, if possible. I'm not sure how much of an upheaval to your build that would be though.

@JunTaoLuo
Copy link
Contributor Author

I'll give that a try. Thanks @nguerrera @dsplaisted !

@JunTaoLuo
Copy link
Contributor Author

Seems like compiling ref assemblies is a little tricky. Applying the hacky fix of pinning implementation assembly version first to see if it unblocks us (local testing was promising). I'll work on compiling against ref assemblies in a separate branch.

@@ -36,10 +36,17 @@ This package is an internal implementation of the .NET Core SDK and is not meant

<!-- Reference implementation assemblies in addition to ref assemblies to get xml docs -->
<ReferenceImplementationAssemblies>true</ReferenceImplementationAssemblies>
<!-- We are ignoring MSB3243 warnings since implemenation and reference assemblies are versioned differently. We need both to compose the targeting pack with reference assemblies and xml docs. -->
<MSBuildWarningsAsMessages>MSB3243</MSBuildWarningsAsMessages >
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this workaround if possible. Should not be necessary since everything has the same assembly versions now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll want to eventually take the approach of building against ref assemblies which means this will come back. Also, I don't want to respin the builds again. But I'll definitely file a follow up issue for further work and mention it there.

@JunTaoLuo
Copy link
Contributor Author

I've verified the contents of the targeting pack of the internal build and it looks good (zip, M.A.App.Ref, tar.gz, deb, rpm, msi). Merging since this is approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-platform Deprecated: Cross-cutting issues related to ASP.NET Core as a platform Servicing-consider Shiproom approval is required for the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants