Copilot commented on code in PR #4556:
URL: https://github.com/apache/solr/pull/4556#discussion_r3486799626


##########
dev-tools/scripts/smokeTestRelease.py:
##########
@@ -282,31 +282,36 @@ def checkSigs(urlString, version, tmpDir, isSigned, 
keysFile):
   if expected != actual:
     raise RuntimeError('solr: wrong artifacts: expected %s but got %s' % 
(expected, actual))
 
-  # Set up clean gpg world; import keys file:
   gpgHomeDir = '%s/solr.gpg' % tmpDir
-  if os.path.exists(gpgHomeDir):
-    shutil.rmtree(gpgHomeDir)
-  os.makedirs(gpgHomeDir, 0o700)
-  gpgLogFile = '%s/solr.gpg.import.log' % tmpDir
-  run('gpg --homedir %s --import %s' % (gpgHomeDir, keysFile), gpgLogFile)
+  if isSigned:
+    # Set up clean gpg world; import keys file:
+    if os.path.exists(gpgHomeDir):
+      shutil.rmtree(gpgHomeDir)
+    os.makedirs(gpgHomeDir, 0o700)
+    gpgLogFile = '%s/solr.gpg.import.log' % tmpDir
+    run('gpg --homedir %s --import %s' % (gpgHomeDir, keysFile), gpgLogFile)
 
   logfile = '%s/solr.assertions.log' % tmpDir
 
   if mavenURL is None:
-    stopGpgAgent(gpgHomeDir, logfile)
+    if isSigned:
+      stopGpgAgent(gpgHomeDir, logfile)
     raise RuntimeError('solr is missing maven')
 
   if dockerURL is None:
-    stopGpgAgent(gpgHomeDir, logfile)
+    if isSigned:
+      stopGpgAgent(gpgHomeDir, logfile)
     raise RuntimeError('solr is missing docker')
 
   if changesURL is None:
-    stopGpgAgent(gpgHomeDir, logfile)
+    if isSigned:
+      stopGpgAgent(gpgHomeDir, logfile)
     raise RuntimeError('solr is missing changes-%s' % version)
   testChanges(version, changesURL)
 
   if openApiURL is None:
-    stopGpgAgent(gpgHomeDir, logfile)
+    if isSigned:
+      stopGpgAgent(gpgHomeDir, logfile)
     raise RuntimeError('solr is missing OpenAPI specification' % version)

Review Comment:
   The RuntimeError message for a missing OpenAPI directory uses string 
formatting without a %s placeholder, which will raise a TypeError and mask the 
real failure.



##########
dev-tools/scripts/smokeTestRelease.py:
##########
@@ -598,123 +601,100 @@ def is_in_list(in_folder, files, indent=4):
       raise RuntimeError('file "%s" is missing' % file_name)
 
 
-def verifyUnpacked(java, artifact, unpackPath, gitRevision, version, testArgs):
+def verifySrcUnpacked(java, artifact, unpackPath, version, testArgs):
+  # The source release is everything in source control minus excluded paths, 
so we don't
+  # check that it has specific things — the build will fail if something 
critical is missing.
+  # Also the binary release checks for presence of things that come from 
source control, so we're
+  # covered that way too.
+  os.chdir(unpackPath)
+  print("  %s" % artifact)
+
+  print('    make sure no JARs/WARs in src dist...')
+  lines = os.popen('find . -name \\*.jar').readlines()
+  if len(lines) != 0:
+    print('    FAILED:')
+    for line in lines:
+      print('      %s' % line.strip())
+    raise RuntimeError('source release has JARs...')
+  lines = os.popen('find . -name \\*.war').readlines()
+  if len(lines) != 0:
+    print('    FAILED:')
+    for line in lines:
+      print('      %s' % line.strip())
+    raise RuntimeError('source release has WARs...')
+
+  validateCmd = './gradlew --no-daemon check -p solr/documentation'
+  print('    run "%s"' % validateCmd)
+  java.run_java(validateCmd, '%s/validate.log' % unpackPath)
+
+  print("    run tests w/ Java %s and testArgs='%s'..." % (BASE_JAVA_VERSION, 
testArgs))
+  java.run_java('./gradlew --no-daemon test %s' % testArgs, '%s/test.log' % 
unpackPath)
+  print("    run integration tests w/ Java %s" % BASE_JAVA_VERSION)
+  java.run_java('./gradlew --no-daemon integrationTest -Dversion.release=%s' % 
version, '%s/itest.log' % unpackPath)
+  print("    build binary release w/ Java %s" % BASE_JAVA_VERSION)
+  java.run_java('./gradlew --no-daemon dev -Dversion.release=%s' % version, 
'%s/assemble.log' % unpackPath)
+  testSolrExample("%s/solr/packaging/build/dev" % unpackPath, java.java_home, 
False)
+
+  if java.run_alt_javas:
+    for run_alt_java, alt_java_version in zip(java.run_alt_javas, 
java.alt_java_versions):
+      print("    run tests w/ Java %s and testArgs='%s'..." % 
(alt_java_version, testArgs))
+      run_alt_java('./gradlew --no-daemon clean test %s' % testArgs, 
'%s/test-java%s.log' % (unpackPath, alt_java_version))
+      print("    run integration tests w/ Java %s" % alt_java_version)
+      run_alt_java('./gradlew --no-daemon integrationTest 
-Dversion.release=%s' % version, '%s/itest-java%s.log' % (unpackPath, 
alt_java_version))
+      print("    build binary release w/ Java %s" % alt_java_version)
+      run_alt_java('./gradlew --no-daemon dev -Dversion.release=%s' % version, 
'%s/assemble-java%s.log' % (unpackPath, alt_java_version))
+      testSolrExample("%s/solr/packaging/build/dev" % unpackPath, 
run_alt_java, False)

Review Comment:
   In the alt-Java loop, testSolrExample is passed run_alt_java (a runner 
function) instead of the alt JAVA_HOME path, which breaks example startup for 
alt-Java runs. Use java.alt_java_homes when setting JAVA_HOME/Path for 
testSolrExample.



##########
dev-tools/scripts/smokeTestRelease.py:
##########
@@ -598,123 +601,100 @@ def is_in_list(in_folder, files, indent=4):
       raise RuntimeError('file "%s" is missing' % file_name)
 
 
-def verifyUnpacked(java, artifact, unpackPath, gitRevision, version, testArgs):
+def verifySrcUnpacked(java, artifact, unpackPath, version, testArgs):
+  # The source release is everything in source control minus excluded paths, 
so we don't
+  # check that it has specific things — the build will fail if something 
critical is missing.
+  # Also the binary release checks for presence of things that come from 
source control, so we're
+  # covered that way too.
+  os.chdir(unpackPath)
+  print("  %s" % artifact)
+
+  print('    make sure no JARs/WARs in src dist...')
+  lines = os.popen('find . -name \\*.jar').readlines()
+  if len(lines) != 0:
+    print('    FAILED:')
+    for line in lines:
+      print('      %s' % line.strip())
+    raise RuntimeError('source release has JARs...')
+  lines = os.popen('find . -name \\*.war').readlines()
+  if len(lines) != 0:
+    print('    FAILED:')
+    for line in lines:
+      print('      %s' % line.strip())
+    raise RuntimeError('source release has WARs...')
+
+  validateCmd = './gradlew --no-daemon check -p solr/documentation'
+  print('    run "%s"' % validateCmd)
+  java.run_java(validateCmd, '%s/validate.log' % unpackPath)
+
+  print("    run tests w/ Java %s and testArgs='%s'..." % (BASE_JAVA_VERSION, 
testArgs))
+  java.run_java('./gradlew --no-daemon test %s' % testArgs, '%s/test.log' % 
unpackPath)
+  print("    run integration tests w/ Java %s" % BASE_JAVA_VERSION)
+  java.run_java('./gradlew --no-daemon integrationTest -Dversion.release=%s' % 
version, '%s/itest.log' % unpackPath)
+  print("    build binary release w/ Java %s" % BASE_JAVA_VERSION)
+  java.run_java('./gradlew --no-daemon dev -Dversion.release=%s' % version, 
'%s/assemble.log' % unpackPath)
+  testSolrExample("%s/solr/packaging/build/dev" % unpackPath, java.java_home, 
False)
+
+  if java.run_alt_javas:
+    for run_alt_java, alt_java_version in zip(java.run_alt_javas, 
java.alt_java_versions):
+      print("    run tests w/ Java %s and testArgs='%s'..." % 
(alt_java_version, testArgs))
+      run_alt_java('./gradlew --no-daemon clean test %s' % testArgs, 
'%s/test-java%s.log' % (unpackPath, alt_java_version))
+      print("    run integration tests w/ Java %s" % alt_java_version)
+      run_alt_java('./gradlew --no-daemon integrationTest 
-Dversion.release=%s' % version, '%s/itest-java%s.log' % (unpackPath, 
alt_java_version))
+      print("    build binary release w/ Java %s" % alt_java_version)
+      run_alt_java('./gradlew --no-daemon dev -Dversion.release=%s' % version, 
'%s/assemble-java%s.log' % (unpackPath, alt_java_version))
+      testSolrExample("%s/solr/packaging/build/dev" % unpackPath, 
run_alt_java, False)
+
+  testChangelogMd('.', version)
+
+
+def verifyBinaryUnpacked(java, artifact, unpackPath, version, gitRevision):
   global SOLR_NOTICE
   global SOLR_LICENSE
 
   os.chdir(unpackPath)
-  isSrc = artifact.find('-src') != -1
-  isSlim = artifact.find('-slim') != -1
-
-  # Check text files in release
+  isSlim = '-slim' in artifact
   print("  %s" % artifact)
-  in_root_folder = list(filter(lambda x: x[0] != '.', os.listdir(unpackPath)))
-  in_solr_folder = []
-  if isSrc:
-    in_solr_folder.extend(os.listdir(os.path.join(unpackPath, 'solr')))
-    is_in_list(in_root_folder, ['LICENSE.txt', 'NOTICE.txt', 'README.md', 
'CONTRIBUTING.md', 'CHANGELOG.md'])
-    is_in_list(in_solr_folder, ['README.adoc'])
-  else:
-    is_in_list(in_root_folder, ['LICENSE.txt', 'NOTICE.txt', 'README.txt', 
'CHANGELOG.md'])
 
   if SOLR_NOTICE is None:
     SOLR_NOTICE = open('%s/NOTICE.txt' % unpackPath, encoding='UTF-8').read()
   if SOLR_LICENSE is None:
     SOLR_LICENSE = open('%s/LICENSE.txt' % unpackPath, encoding='UTF-8').read()
 
-  # if not isSrc:
-  #   # TODO: we should add verifyModule/verifySubmodule (e.g. analysis) here 
and recurse through
-  #   expectedJARs = ()
-  #
-  #   for fileName in expectedJARs:
-  #     fileName += '.jar'
-  #     if fileName not in l:
-  #       raise RuntimeError('solr: file "%s" is missing from artifact %s' % 
(fileName, artifact))
-  #     in_root_folder.remove(fileName)
-
-  if isSrc:
-    expected_src_root_folders = ['build-tools', 'changelog', 'dev-docs', 
'dev-tools', 'gradle', 'solr']
-    expected_src_root_files = ['build.gradle', 'gradlew', 'gradlew.bat', 
'settings.gradle', 'settings-gradle.lockfile', 'versions.lock']
-    expected_src_solr_files = ['build.gradle']
-    expected_src_solr_folders = ['benchmark',  'bin', 'modules', 'api', 
'core', 'cross-dc-manager', 'docker', 'documentation', 'example', 'licenses', 
'packaging', 'distribution', 'server', 'solr-ref-guide', 'solrj', 
'solrj-jetty', 'solrj-streaming', 'solrj-zookeeper', 'test-framework', 
'webapp', '.gitignore', '.gitattributes']
-    is_in_list(in_root_folder, expected_src_root_folders)
-    is_in_list(in_root_folder, expected_src_root_files)
-    is_in_list(in_solr_folder, expected_src_solr_folders)
-    is_in_list(in_solr_folder, expected_src_solr_files)
-    if len(in_solr_folder) > 0:
-      raise RuntimeError('solr: unexpected files/dirs in artifact %s solr/ 
folder: %s' % (artifact, in_solr_folder))
-  elif isSlim:
+  in_root_folder = list(filter(lambda x: x[0] != '.', os.listdir(unpackPath)))
+  is_in_list(in_root_folder, ['LICENSE.txt', 'NOTICE.txt', 'README.txt', 
'CHANGELOG.md'])
+
+  if isSlim:
     is_in_list(in_root_folder, ['bin', 'docker', 'docs', 'example', 
'licenses', 'server', 'lib'])
   else:
     is_in_list(in_root_folder, ['bin', 'modules', 'cross-dc-manager', 
'docker', 'docs', 'example', 'licenses', 'server', 'lib'])
 
   if len(in_root_folder) > 0:
     raise RuntimeError('solr: unexpected files/dirs in artifact %s: %s' % 
(artifact, in_root_folder))
 
-  if isSrc:
-    print('    make sure no JARs/WARs in src dist...')
-    lines = os.popen('find . -name \\*.jar').readlines()
-    if len(lines) != 0:
-      print('    FAILED:')
-      for line in lines:
-        print('      %s' % line.strip())
-      raise RuntimeError('source release has JARs...')
-    lines = os.popen('find . -name \\*.war').readlines()
-    if len(lines) != 0:
-      print('    FAILED:')
-      for line in lines:
-        print('      %s' % line.strip())
-      raise RuntimeError('source release has WARs...')
-
-    validateCmd = './gradlew --no-daemon check -p solr/documentation'
-    print('    run "%s"' % validateCmd)
-    java.run_java(validateCmd, '%s/validate.log' % unpackPath)
-
-    print("    run tests w/ Java %s and testArgs='%s'..." % 
(BASE_JAVA_VERSION, testArgs))
-    java.run_java('./gradlew --no-daemon test %s' % testArgs, '%s/test.log' % 
unpackPath)
-    print("    run integration tests w/ Java %s" % BASE_JAVA_VERSION)
-    java.run_java('./gradlew --no-daemon integrationTest -Dversion.release=%s' 
% version, '%s/itest.log' % unpackPath)
-    print("    build binary release w/ Java %s" % BASE_JAVA_VERSION)
-    java.run_java('./gradlew --no-daemon dev -Dversion.release=%s' % version, 
'%s/assemble.log' % unpackPath)
-    testSolrExample("%s/solr/packaging/build/dev" % unpackPath, 
java.java_home, False)
-
-    if java.run_alt_javas:
-      for run_alt_java, alt_java_version in zip(java.run_alt_javas, 
java.alt_java_versions):
-        print("    run tests w/ Java %s and testArgs='%s'..." % 
(alt_java_version, testArgs))
-        run_alt_java('./gradlew --no-daemon clean test %s' % testArgs, 
'%s/test-java%s.log' % (unpackPath, alt_java_version))
-        print("    run integration tests w/ Java %s" % alt_java_version)
-        run_alt_java('./gradlew --no-daemon integrationTest 
-Dversion.release=%s' % version, '%s/itest-java%s.log' % (unpackPath, 
alt_java_version))
-        print("    build binary release w/ Java %s" % alt_java_version)
-        run_alt_java('./gradlew --no-daemon dev -Dversion.release=%s' % 
version, '%s/assemble-java%s.log' % (unpackPath, alt_java_version))
-        testSolrExample("%s/solr/packaging/build/dev" % unpackPath, 
run_alt_java, False)
-
-  else:
-    # Binary tarball
-    checkAllJARs(os.getcwd(), gitRevision, version)
-
-    print('    copying unpacked distribution for Java %s ...' % 
BASE_JAVA_VERSION)
-    javaBaseVersionUnpackPath = '%s-java%s' % (unpackPath, BASE_JAVA_VERSION)
-    if os.path.exists(javaBaseVersionUnpackPath):
-      shutil.rmtree(javaBaseVersionUnpackPath)
-    shutil.copytree(unpackPath, javaBaseVersionUnpackPath)
-    os.chdir(javaBaseVersionUnpackPath)
-    print('    test solr example w/ Java %s...' % BASE_JAVA_VERSION)
-    testSolrExample(javaBaseVersionUnpackPath, java.java_home, isSlim)
-
-    if java.run_alt_javas:
-      for run_alt_java, alt_java_version in zip(java.run_alt_javas, 
java.alt_java_versions):
-        print("The alt version of java", run_alt_java, alt_java_version)
-        print('    copying unpacked distribution for Java %s ...' % 
alt_java_version)
-        javaAltVersionUnpackPath = '%s-java%s' % (unpackPath, alt_java_version)
-        if os.path.exists(javaAltVersionUnpackPath):
-          shutil.rmtree(javaAltVersionUnpackPath)
-        shutil.copytree(unpackPath, javaAltVersionUnpackPath)
-        os.chdir(javaAltVersionUnpackPath)
-        print('    test solr example w/ Java %s...' % alt_java_version)
-        testSolrExample(javaAltVersionUnpackPath, run_alt_java, isSlim)
-
-    os.chdir(unpackPath)
+  checkAllJARs(os.getcwd(), gitRevision, version)
+
+  print('    copying unpacked distribution for Java %s ...' % 
BASE_JAVA_VERSION)
+  javaBaseVersionUnpackPath = '%s-java%s' % (unpackPath, BASE_JAVA_VERSION)
+  if os.path.exists(javaBaseVersionUnpackPath):
+    shutil.rmtree(javaBaseVersionUnpackPath)
+  shutil.copytree(unpackPath, javaBaseVersionUnpackPath)
+  os.chdir(javaBaseVersionUnpackPath)
+  print('    test solr example w/ Java %s...' % BASE_JAVA_VERSION)
+  testSolrExample(javaBaseVersionUnpackPath, java.java_home, isSlim)
+
+  if java.run_alt_javas:
+    for run_alt_java, alt_java_version in zip(java.run_alt_javas, 
java.alt_java_versions):
+      print('    copying unpacked distribution for Java %s ...' % 
alt_java_version)
+      javaAltVersionUnpackPath = '%s-java%s' % (unpackPath, alt_java_version)
+      if os.path.exists(javaAltVersionUnpackPath):
+        shutil.rmtree(javaAltVersionUnpackPath)
+      shutil.copytree(unpackPath, javaAltVersionUnpackPath)
+      os.chdir(javaAltVersionUnpackPath)
+      print('    test solr example w/ Java %s...' % alt_java_version)
+      testSolrExample(javaAltVersionUnpackPath, run_alt_java, isSlim)

Review Comment:
   In the alt-Java loop, testSolrExample is passed run_alt_java (a runner 
function) instead of the alt JAVA_HOME path, so JAVA_HOME/PATH are set to a 
function repr and the example launch will fail. Use java.alt_java_homes when 
calling testSolrExample.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to