Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#2956 closed defect (fixed)

jshydra does not ensure that correct jsshell version is present

Reported by: sergz Assignee: kzar
Priority: P3 Milestone:
Module: Automation Keywords: jshydra, jsshell
Cc: eric, oleksandr, fhd, kzar Blocked By:
Blocking: #3344 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29330389/

Description (last modified by kzar)

Description

The jshdyra repository downloads a copy of jsshell if it is not already present in the mozilla/ directory, but unfortunately it does not check that the version of jsshell is correct. This means that if an old version of jsshell is already present, the new version will not be downloaded. The old version of jsshell will then be used instead, and that can cause problems.

The version of jsshell can be checked with a command like mozilla/js --help | grep -i version, the ensureJsShell() function in utils.py needs to check the version matches what is expected. If the version doesn't match jsshell should be re-downloaded.

Change History (9)

comment:1 Changed 4 years ago by kzar

Hi sergz, I don't fully understand the problem. I have had no problems with multiple levels of nested dependencies personally.

Please could you provide a concrete list of steps I can follow to reproduce this? (Along with expected vs observed commit references.) Words like "updating" and "obsolete" are ambiguous in this context.

Sorry to be a pain, Dave.

comment:2 Changed 4 years ago by kzar

  • Cc kzar added

comment:3 Changed 4 years ago by oleksandr

Here's how I reproduce it:

  • take adblockpluschrome repository. Go to some old version. Say 1.8.12 (37cfc2b4340c).
  • Delete the buildtools folder.
  • run "build.py -t chrome devenv". Observe the checkouts.
  • Then go to the tip and run "build.py -t chrome devenv" again.
  • Observe an error because of incorrect repository version.
  • Delete buildtools and run "build.py -t chrome devenv" again. All good then.

comment:4 Changed 4 years ago by kzar

I still can't reproduce this guys. First I try to reproduce Ollie's steps:

git clone git@github.com:adblockplus/adblockpluschrome.git
cd adblockpluschrome
git checkout 1.8.12
./ensure_dependencies.py
git checkout master
./ensure_dependencies.py
find -name .git -exec dirname {} \; -exec sh -c 'git -C $(dirname {}) rev-parse HEAD' \; | xargs -n2
./buildtools c5a8f48c65f2d09774582ac6cb6697bd5b3159e3
./buildtools/jshydra fd2605c2f0b1e4eb230166a8ca7078d62b83c922
. fefdb7f8069662c3ee2d4f2cb03e4dcfdd802364
./adblockplus/buildtools c5a8f48c65f2d09774582ac6cb6697bd5b3159e3
./adblockplus/buildtools/jshydra fd2605c2f0b1e4eb230166a8ca7078d62b83c922
./adblockplus 2af0272e9ac4cc2956594f11ec4e23d7cea6b447
./adblockplus/adblockplusui 4633d20ca3940981749d40a86a5db03c9a346864
./adblockplusui 4633d20ca3940981749d40a86a5db03c9a346864
./adblockplustests/buildtools 021b78c35e9e19a81f6a3eb1449d868b4cfb828a
./adblockplustests/buildtools/jshydra 2bdc6ce262571888bafa91cf867e239a80839429
./adblockplustests a366bd9ce122042800320538e1418b56cce50029

Then I try from a fresh clone:

git clone git@github.com:adblockplus/adblockpluschrome.git
cd adblockpluschrome
./ensure_dependencies.py
find -name .git -exec dirname {} \; -exec sh -c 'git -C $(dirname {}) rev-parse HEAD' \; | xargs -n2
./buildtools c5a8f48c65f2d09774582ac6cb6697bd5b3159e3
./buildtools/jshydra fd2605c2f0b1e4eb230166a8ca7078d62b83c922
. fefdb7f8069662c3ee2d4f2cb03e4dcfdd802364
./adblockplus/buildtools c5a8f48c65f2d09774582ac6cb6697bd5b3159e3
./adblockplus/buildtools/jshydra fd2605c2f0b1e4eb230166a8ca7078d62b83c922
./adblockplus 2af0272e9ac4cc2956594f11ec4e23d7cea6b447
./adblockplus/adblockplusui 4633d20ca3940981749d40a86a5db03c9a346864
./adblockplusui 4633d20ca3940981749d40a86a5db03c9a346864
./adblockplustests/buildtools 021b78c35e9e19a81f6a3eb1449d868b4cfb828a
./adblockplustests/buildtools/jshydra 2bdc6ce262571888bafa91cf867e239a80839429
./adblockplustests a366bd9ce122042800320538e1418b56cce50029

It appears that the correct revisions for all the dependencies have been checked out both times.

Please can you explain again how I can reproduce this? (I can't use the build script at the moment as I'm having problems connecting to Mozilla's FTP server for the jsshell download. Besides it is better to issolate the problem to ensure_dependencies.py)

comment:5 Changed 4 years ago by oleksandr

All repositories are actually updated fine. The problem is that JSHydra doesn't rewrite the mozilla directory after the update. Looks like abprewrite.js assumes all is fine if the mozilla directory already there.

comment:6 Changed 4 years ago by kzar

  • Description modified (diff)
  • Keywords jshydra jsshell added
  • Owner set to kzar
  • Priority changed from Unknown to P3
  • Ready set
  • Summary changed from ensure_dependencies.py does not update children of already updated children. to jshydra does not ensure that correct jsshell version is present

comment:7 Changed 4 years ago by kzar

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:8 Changed 4 years ago by kzar

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:9 Changed 4 years ago by kzar

  • Blocking 3344 added
Note: See TracTickets for help on using tickets.