Skip to content
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

package.json conflicts when adding to OpenUPM #30

Closed
shniqq opened this issue Oct 15, 2020 · 10 comments
Closed

package.json conflicts when adding to OpenUPM #30

shniqq opened this issue Oct 15, 2020 · 10 comments
Labels
enhancement New feature or request

Comments

@shniqq
Copy link

shniqq commented Oct 15, 2020

Hi, I wanted to add this package to the OpenUPM registry, but as mentioned here there are some issues with the fact that there are two package.json files in this project.

As discussed over there, this package has been published already, and adjusting the package.json to be compatible with OpenUPM might/will break the existing way of publishing it. In my opinion OpenUPM is the more "standard" way of distributing a Unity-specific package compared to the npmjs registry - any chance the suggested changes could be made?

Could also attempt to make the changes myself, but wanted to discuss with the maintainers here first before digging into it.

Cheers!

@favoyang
Copy link

Hi @ashblue,

Thanks for creating this project. I speak for OpenUPM, a platform collects and builds open-source UPM packages. Basically, we monitor Git Tags and create versioned UPM there, then the developer can install the package from our registry.

I won't say it's a standard way (thanks for @shniqq), but I'd like to help to find a workaround to make this package available on the platform.

  1. The duplicated package.json. One located at the root path, one at the Assets folder. It is a bit confusing because they share the same name com.fluid.behavior-tree. I'd like to change the root one to a different name like behavior-tree-project. But it is already published on the npmjs, maybe you're unwilling to change it. BTW, I noticed it's required by your tool https://www.npmjs.com/package/upm-package-populator. Anyway, this is a minimal issue confused by our pipelines build script, but it has a workaround (see the end).

  2. The version field is not get updated, it is always 0.0.0. It should be synced with Git tags name. So either you maintain it in the master branch, or your build tool generates it in a Git tag.

I propose a workaround to fix all these two issues at once, by adding an another publish script like the publish-nightly.sh. Basically, when you publish a new version, you run the new script to push the (built) dist folder to a new branch named upm/v.x.y.z.

@ashblue ashblue added the enhancement New feature or request label Oct 17, 2020
@ashblue
Copy link
Owner

ashblue commented Oct 17, 2020

Hello @favoyang and @shniqq,

First off thank you both, I always appreciate constructive criticism. My project (and several others) runs on a Unity package generation system called Oyster. Right now my system deploys several automated things with versioning built around NPM (which was the only available system at the time for this sorta thing). That said, let me sum up what I might be able to do in the future based on your feedback for OUPM.

  1. Renaming the root package.json isn't a bad idea. I'd have to look into re-routing the package and some other things that could blow out older versions. I can definitely consider this and it's a good idea. I'd also have to modify my package generator too to generate a separate name. This also might create confusion when implementing the package from NPM (not sure).
  2. The version in the package.json should be updated to the Semantic Release version in master so that is a legit bug. Separate ticket opened here: Semantic Release is not updating the master branch version #32
  3. Currently tags are automatically generated as expected by Semantic Release (see tags on home page). I'm also not keen on adding branches since tags are already there. Is it possible for me to just toss the built version of the package to you via an endpoint during my build scripts, eat tags, detect Git releases, or eat the version number when I fix it? Would be even cooler if there was a simple NPM package I could install and run that would send over the built package to you at the end of my processes.

Considering the above it might be a while before I can address this and I might close the issue if it goes stale before I can get to it. That said I have entered a ticket into my backlog for this. I'm working on a large telehealth project connected to the pandemic atm, so my time has been a little limited to provide new features. Won't always be the case though.

In short, if there are some minor changes you need to make to the build scripts I'm more than happy to pull in a merge request :) If not might be a little while till I get to this.

@favoyang
Copy link

Renaming the root package.json isn't a bad idea.

If you're okay about this, then the future tagged version (new Git tag), will contain only one package.json with the name com.fluid.behavior-tree. Our build pipelines will be happy to locate the right one and serve it.

The version in the package.json should be updated to the Semantic Release version in master so that is a legit bug. Separate ticket opened here: #32

Good to know it's a bug.

Currently tags are automatically generated as expected by Semantic Release (see tags on home page). I'm also not keen on adding branches since tags are already there.

If you're okay to rename the root package.json name then you don't need to do this at all. All I proposed before is the workaround based on not updating the root package.json name.

Just in case you want to learn about how to add extra stuff to work with Semantic Release, I wrote an article before: https://medium.com/openupm/how-to-maintain-upm-package-part-2-f352fbf5f87c#6bf4. It basically runs semantic release, then a custom script, then tags it as a special versioned tag.

@ashblue
Copy link
Owner

ashblue commented Mar 23, 2021

@favoyang sorry for the delayed response. Finally got around to upgrading my underlying package generator code and this came up (my day job is COVID19 related, so I've been crazy busy). It sounds like I just need to fix the package.json versioning on master (looks like that might be a bit more complicated than I realized).

For the package.json location can I simply put a .openupm config in my root with something like { packageRoot; "RELATIVE_LOCATION" }. I saw this https://openupm.com/docs/adding-upm-package.html#package-yaml-file but I wasn't sure how to include the file in my project (didn't see a file name). Also, I didn't see anything in there about declaring the package root.

Trying to tidy things up with the underlying package generator so it just makes OUPM friendly packages with zero effort (https://github.com/ashblue/oyster-package-generator).

@favoyang
Copy link

@ashblue I revisited the thread. Basically, you have two package.json files with the same package name located in different folders. OpenUPM build pipelines just checkout your repository and search for the package.json with a given package name. In this case, it will pick the one at the root path. We don't support the packageRoot configuration at the moment, because we don't need it.

The package YAML file you mentioned is located at the OpenUPM repository's curated list folder https://github.com/openupm/openupm/tree/master/data/packages. You can create this file (PR) by using our helper form: https://openupm.com/packages/add/. There's already a PR at openupm/openupm#1491 waiting to merge.

To make clear, you are suggested to,

Just let me know if this works for you. Then I will merge openupm/openupm#1491.

@ashblue
Copy link
Owner

ashblue commented Mar 24, 2021

Sounds good. I have to preserve the package.json in master. But I can create a build branch that always has the latest built files in the root (with the correct version number) if that works? That way it strips out the double package.json issue you were mentioning. Same idea as the publish nightly you mentioned earlier on.

https://github.com/ashblue/fluid-behavior-tree/blob/master/publish-nightly.sh

PS Thanks for your patience on this. I'm sure you're super busy running a huge tool like OUPM.

@favoyang
Copy link

It will work as well. Notice that OpenUPM won't track Git tags. So you need to prepare a separated Git tag from the build branch. i.e.

  • v2.2.2 tagged from the main branch
  • upm/v2.2.2 tagged from the build branch

@ashblue
Copy link
Owner

ashblue commented Mar 24, 2021

Hmm, I think the build would overwrite the tag if it does the same thing as the nightly (force pushes over it). Not sure though (it might keep the commit hash with the tag).

@favoyang
Copy link

I think all you need to do is after running the publish-nightly.sh command, test if the semantic-release generates a new release, then create a separate upm/ prefixed git tag based on the build branch. It will require reading the output of - npm run semantic-release, you may need to research how to do that on your CI.

I wrote a blog doing similar things, but that's based on GitHub actions - https://medium.com/openupm/how-to-maintain-upm-package-part-2-f352fbf5f87c#6bf4

    - name: Create upm git tag
      if: steps.semantic.outputs.new_release_published == 'true'
      run: |
        git tag $TAG upm
        git push origin --tags
      env:
        TAG: upm/v${{ steps.semantic.outputs.new_release_version }}

@ashblue
Copy link
Owner

ashblue commented Jun 2, 2021

This is technically a feature request and not an open issue. So I'm moving this to a discussion.

#62

@ashblue ashblue closed this as completed Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants