phetsims / perennial

Maintenance tools that won't change with different versions of chipper checked out
MIT License
3 stars 5 forks source link

perennial, perennial-alias and pull-all script #289

Closed veillette closed 1 year ago

veillette commented 1 year ago

While trying out GitHub codespace, I checked out all the repos to build calculus-grapher, as if I was a new user. Although Calculus-Grapher is not published, I tried to mimic the steps given for a published simulation such as https://github.com/phetsims/gravity-and-orbits/edit/master/README.md

If you do some, you would clone perennial as git clone https://github.com/phetsims/perennial.git perennial-alias

I don't know the history of perennial and perennial-alias, but I ran into a problem when I wanted to update the repos. If you try to do a pull-all on all repos, it fails because it tries to look up a file in perennial directory instead of perennial-alias.

# pull each repository
for repo in `cat perennial/data/active-repos | xargs | tr -d '\r'`
do
  if [ -d "${repo}" ]; then
    echo ${repo}
    cd ${repo} > /dev/null
    if [ ${parallel} == "true" ]; then
      # run in the background
      ${pullCommand} &
    else
      ${pullCommand}
    fi
    cd ..
  fi
done

if [ ${parallel} == "true" ]; then
  # wait for all background tasks to complete
  wait
fi
brettfiedler commented 1 year ago

Possibly naive question: Is the readme incorrect? Is it missing an extra line before git clone https://github.com/phetsims/perennial.git perennial-alias that reads git clone https://github.com/phetsims/perennial.git so there are two copies checked out?

Or is this a deeper problem with pull-all and the instructions in the development overview?

veillette commented 1 year ago

Yes, checking out both copies would fix the problem. So including them in the readme would address the problem for a newcomer. However I don't know if it is the appropriate solution, or it points to a deeper problem in perennial.

For instance chipper has this as bad-text

// see getBadTextTester for schema.
  const forbiddenTextObjects = [

    // chipper should use perennial-alias instead of perennial, so that it can check out specific versions
    '../perennial/js/'
  ];

but the repo perennial.git makes multiples references to both perennial-alias and perennial; I personally don't understand the division.

Someone such as @samreid or @jonathanolson would know more of the context.

jonathanolson commented 1 year ago

They both have the same content, as perennial-alias is essentially just a renamed copy of perennial (that will pull from perennial).

We want those functions and utilities, however perennial is named as such since it should always stay on master (e.g. maintenance tooling), whereas sim release branches might need to access some of that content, and will use perennial-alias (which does NOT need to be on master, and will have SHAs taken for release branches).

So if you're writing code in sims or chipper, generally perennial-alias should be used.

No clue why an update wouldn't work, perhaps we could look into it together?

veillette commented 1 year ago

The readmes in the published simulations include instructions such as

git clone https://github.com/phetsims/perennial.git perennial-alias but does not include git clone https://github.com/phetsims/perennial.git perennial

@BLFiedler was suggesting that we include both

git clone https://github.com/phetsims/perennial.git perennial-alias git clone https://github.com/phetsims/perennial.git perennial

This would get around the problem illustrated in https://github.com/phetsims/perennial/issues/289#issue-1455829504. This is important for the POSE, that would start developing a simulation by following instructions in the readmes.

What do you think @jonathanolson ?

jonathanolson commented 1 year ago

Ahhh, I definitely didn't understand this was when you do NOT have perennial checked out!

Those scripts mainly meant for people who have everything checked out, I'd encourage people to generally just pull the repos using an IDE, or a normal way of pulling the checked out repos, instead of using a perennial-alias script.

veillette commented 1 year ago

Then there is nothing to do here. We will let people know not to use those scripts if they dont have all the repos checked out. We can close this issue.