[New port] Added Sciter.JS port (version 4.4.8.16)

[VuYeK]

Describe the pull request

• What does your PR fix?

  Add new Sciter.JS port (https://github.com/c-smile/sciter-js-sdk)

• Which triplets are supported/not supported? Have you updated the CI baseline?

  all, No

• Does your PR follow the maintainer guide?

  Yes

• If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

  Yes

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/

[BillyONeal]

I'm sorry about the lengthy osx rebuild, I messed that up in our builds :(

[BillyONeal]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[VuYeK]

Finally OSX build ended 😅

[BillyONeal]

Finally OSX build ended 😅

Yeah I accidentally deleted the Azure Storage account where all the caches were stored for the osx fleet 😳

[JonLiu1993]

@BillyONeal ,Could you please help review this pr?

[BillyONeal]

Mostly "request changes" over discussion on whether the DLL there is safe to use from a debug context.

[JonLiu1993]

@VuYeK ，Could you please clarify the review comments above

[VuYeK]

@VuYeK ，Could you please clarify the review comments above

Done.

[JonLiu1993]

@BillyONeal ,Could you please help review this pr?

[BillyONeal]

I'm still nervous about the debug thing but OK. I'm pretty sure the directory suffix should come off though..

[BillyONeal]

Because this conflicts with an existing sciter port, I don't think we can list it in the vcpkg catalog. Consider creating a community registry to contain the port at which you can point your customers.

See #17161 for other community registries.

(Sorry for not noticing this conflict before I was just about to push the approve button :( )

[VuYeK]

Because this conflicts with an existing sciter port, I don't think we can list it in the vcpkg catalog. Consider creating a community registry to contain the port at which you can point your customers.

See #17161 for other community registries.

(Sorry for not noticing this conflict before I was just about to push the approve button :( )

I don't understand why, subdirectory completely solves this problem and allows to have both version installed together. It'll be very disappointing if "older" version will block possibility to use new one. I could "update" the 'sciter' port to 'sciter-js' but it isn't good idea because if someone has 'sciter' as dependency then his app will stop working after this update.

Following your way will cause that vcpkg won't ever have newer version of sciter and will stuck on old non-js version when support will end. Subdirectory allows existing both together.

[BillyONeal]

The problem is that it breaks the contract vcpkg provides to its users -- the way vcpkg avoids needing to have the cross product of build system integrations between ports and users is this "virtual system root" installed tree approach. Binaries must be in bin or integrations downstream won't work. For example, if someone had both ports installed, it is likely they would get the sciter DLL even though they intended to use sciter-js (which would probably explode at runtime).

Following your way will cause that vcpkg won't ever have newer version of sciter and will stuck on old non-js version when support will end. Subdirectory allows existing both together.

That seems unlikely; the same DLL name means that they cannot be deployed together. We can add a new port and skip the old one in CI, but would only want to do that if this is a complete replacement for the old one and not a fork or something like that.

I think the options are one of the following:

• If this is a new / complete replacement for the older port, the older port should be updated to point to this. Users who want the previous thing can pin to it with versioning. Alternatively, we could make the old port print warning messages that it is deprecated in favor of this one, and stop testing that one in CI to resolve the conflict.
• If this is a fork / intended to compete with the other port, then the right behavior is to put it into a community registry as described earlier, and for users to access it with overlays.

[JonLiu1993]

@VuYeK, have you considered BillyONEal’s comments

[VuYeK]

@VuYeK ,Could you please clarify the question? #18951 (comment)

Ok, I have created registry with old TIS version like @BillyONeal wanted to: https://github.com/VuYeK/vcpkg-registry. Additionally I've updated sciter-js version in PR to 4.4.8.16. Can we finally merge and close this request?

[ras0219-msft]

Thanks @VuYeK for your continued work here and sorry for the difficult situation that upstream has put us all in.

I have two changes that are needed before this is mergeable: first, sciter-js is not a replacement for sciter and thus we should not implicitly install it. Second, we need to explain to users what the situation is and make them aware of their options.

sciter will also need to be added to ci.baseline.txt as failing on all platforms.

Once these changes have been made, this PR will be ready to merge.

[github-actions]

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout 6fd6830d68fbab599ad78138d3e6d87dcbcbb016 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/s-/sciter.json b/versions/s-/sciter.json
index 11787b3..1bb7541 100644
--- a/versions/s-/sciter.json
+++ b/versions/s-/sciter.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "450fb94588bfe7a7927c46cbdda5755d8868504d",
+      "git-tree": "f4277322a7ad3982cb864461d5e6c2187cbaa679",
       "version-string": "deprecated",
       "port-version": 0
     },

[VuYeK]

Thanks @VuYeK for your continued work here and sorry for the difficult situation that upstream has put us all in.

I have two changes that are needed before this is mergeable: first, sciter-js is not a replacement for sciter and thus we should not implicitly install it. Second, we need to explain to users what the situation is and make them aware of their options.

sciter will also need to be added to ci.baseline.txt as failing on all platforms.

Once these changes have been made, this PR will be ready to merge.

Ok, all changes has been applied :D

[pravic]

Other than one remark, I agree with this move.

[github-actions]

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout 6fd6830d68fbab599ad78138d3e6d87dcbcbb016 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/s-/sciter.json b/versions/s-/sciter.json
index c455a59..2acb342 100644
--- a/versions/s-/sciter.json
+++ b/versions/s-/sciter.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "942b0c505b2dd4317db0eb419416110c9db024be",
+      "git-tree": "1ff6683a2d398db76e1791b994dc157ffb21e88c",
       "version-string": "deprecated",
       "port-version": 0
     },

[ras0219-msft]

Thanks everyone!

[julianxhokaxhiu]

Is there an example on how to use this package from a CMakeLists.txt POV? I was expecting to find a FindSciterJS.cmake file somewhere but there's none. Is it possible to find SciterJS dependencies through find_package?

//EDIT: I'm currently developing my own port and I'll update this post as soon as it is completed, with a real full integration with vcpkg and CMake find_package.